[Source-postgres] : Compare each record’s lsn_commit value instead of lsn_proc. #35939
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
octavia-squidington-iii
added
area/connectors
akashkulk
changed the title
octavia-squidington-iii
added
the
area/documentation
...res/src/main/java/io/airbyte/integrations/source/postgres/cdc/PostgresCdcTargetPosition.java
Show resolved
Hide resolved
...res/src/main/java/io/airbyte/integrations/source/postgres/cdc/PostgresCdcTargetPosition.java
Show resolved
Hide resolved
| if (!isOnLegacyPostgres()) { | ||
| assertTrue(stateMessagesCDC.size() > 1, "Generated only the final state."); | ||
| } | ||
| // We expect only one cdc state message, as all the records are inserted in a single transaction. Since |
There was a problem hiding this comment.
thank you for the additional comment. Does that mean that somehow, your change is also fixing legacy postgres?
There was a problem hiding this comment.
Not quite. For legacy postgres, only one state message was expected (not sure why).
So this change just streamlines the test to check for just one state message instead of multiple (which I'm not sure was a good test to begin with)
There was a problem hiding this comment.
we NEVER are on legacy postgres... Let's remove that method altogether, no?
| } | ||
| // We expect only one cdc state message, as all the records are inserted in a single transaction. | ||
| // Since | ||
| // lsn_commit only increases with a new transaction, we expect only one state message. |
There was a problem hiding this comment.
are we not expecting a checkpoint state message (after the transaction) and a final state message in this case?
There was a problem hiding this comment.
The one state message we expect is after the dbz engine is shut down. In this case, there is just one transaction so only one state message is expected. (that's the change we're trying to test by comparing against lsn_commit and not lsn_proc)
Closes #35963