🎉Source Postgres: 13608 & 12026 - Align regular and CDC integration tests and data mappers

[yurii-bidiuk]

What

Fixes #13608 #12026 Currently, we have 2 different classes for integration datatype tests CdcMssqlSourceDatatypeTest and MssqlSourceDatatypeTest. They have the same input args, but differ "expected result" for some types. CDC source loses milliseconds precision when a timestamp/datetime is converted to string

How

Updated data mappers for CDC and regular sync modes. Added mappers for datetime datatypes to save millisecond and make datetime/date/time formats the same as for regular sync

Recommended reading order

1. PostgresConverter.java
2. PostgresSourceOperations.java
3. DateTimeConverter.java
4. AbstractPostgresSourceDatatypeTest.java

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

• [ ] Community member? Grant edit access to maintainers (instructions)
• [ ] Secrets in the connector's spec are annotated with airbyte_secret
• [ ] Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
• [ ] Code reviews completed
• [ ] Documentation updated
  • [ ] Connector's README.md
  • [ ] Connector's bootstrap.md. See description and examples
  • [ ] docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • [ ] docs/integrations/README.md
  • [ ] airbyte-integrations/builds.md
• [ ] PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

• [ ] Create a non-forked branch based on this PR and test the below items on it
• [ ] Build is successful
• [ ] If new credentials are required for use in CI, add them to GSM. Instructions.
• [ ] /test connector=connectors/<name> command is passing
• [ ] New Connector version released on Dockerhub by running the /publish command described here
• [ ] After the connector is published, connector added to connector index as described here
• [ ] Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

• [ ] Grant edit access to maintainers (instructions)
• [ ] Secrets in the connector's spec are annotated with airbyte_secret
• [ ] Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
• [ ] Code reviews completed
• [ ] Documentation updated
  • [ ] Connector's README.md
  • [ ] Connector's bootstrap.md. See description and examples
  • [ ] Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
• [ ] PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

• [ ] Create a non-forked branch based on this PR and test the below items on it
• [ ] Build is successful
• [ ] If new credentials are required for use in CI, add them to GSM. Instructions.
• [ ] /test connector=connectors/<name> command is passing
• [ ] New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

• [ ] Issue acceptance criteria met
• [ ] PR name follows PR naming conventions
• [ ] If adding a new generator, add it to the list of scaffold modules being tested
• [ ] The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
• [ ] Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

[yurii-bidiuk]

/test connector=connectors/source-postgres

:clock2: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2654989444 :x: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2654989444 :bug: https://gradle.com/s/vjkkeey3btpw6

Build Failed

Test summary info:

Could not find result summary

[yurii-bidiuk]

/test connector=connectors/source-postgres

:clock2: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2669913180 :x: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2669913180 :bug: https://gradle.com/s/ugohxxhfjemus

Build Failed

Test summary info:

Could not find result summary

[yurii-bidiuk]

/test connector=connectors/source-postgres

:clock2: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2670960204 :white_check_mark: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2670960204 No Python unittests run

Build Passed

Test summary info:

All Passed

[grishick]

I added @subodh1810 and @edgao as reviewers, because they are (have been recently) working on this code

[edgao]

@subodh1810 I've only skimmed this PR (and basically haven't looked into that issue at all), but I think they should be aligned? https://github.com/airbytehq/airbyte/issues/14628 is probably easier if we have unified tests, and hopefully updating these tests to assert the correct values will be sufficient to verify that the issue is done?

[edgao]

actually: for https://github.com/airbytehq/airbyte/issues/14628, doesn't this PR resolve most of it?

• mysql / other DB sources still need to be fixed, but they can be based on this PR
• postgres BCE handling is still broken, but that's maybe a debezium limitation

[edgao]

merged in master + fixed conflicts + enabled catalog assert for the CDC tests (these were already passing, at least on my laptop!)

[edgao]

/test connector=connectors/source-postgres

:clock2: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2749458333 :white_check_mark: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2749458333 No Python unittests run

Build Passed

Test summary info:

All Passed

[subodh1810]

My review should not be a blocker, I just wanted to highlight that might be a mechanism to handle few data types via the data type handling mode flag and if testing the modes is taking a lot of time, feel free to go ahead and merge this and create a separate issue to experiment with the debezium data type handling mode flags

[yurii-bidiuk]

Hi @subodh1810 I made some research on the properties that you provided. A brief result for some of them:

• binary.handling.mode - This could be used instead of our implementation because it could represent bytea as a hex. But there will be a small difference with non-CDC mode because non-CDC mode represents bytea with leading \x (e.g. \x123), and CDC mode with binary.handling.mode: hex represents bytea without \x (e.g. 123). I'm not sure which format is better for us.

• hstore.handling.mode - According to #11953 hstore should be written as json, so now it works well for CDC.

• interval.handling.mode - The same result as for binary.handling.mode: we could use this property but the represented value would be different from the value in the non-CDC mode.

[edgao]

thanks for the analysis! In that case, I think we should publish+merge this PR as-is.

[edgao]

/test connector=connectors/source-postgres

:clock2: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2785534658 :x: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2785534658 :bug: https://gradle.com/s/gfnawrvsw5ocq

Build Failed

Test summary info:

Could not find result summary

:clock2: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2785534658 :white_check_mark: connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2785534658 No Python unittests run

Build Passed

Test summary info:

All Passed

[edgao]

/publish connector=connectors/source-postgres

:clock2: Publishing the following connectors:
connectors/source-postgres
https://github.com/airbytehq/airbyte/actions/runs/2786007129

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres	:x:	:x:

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

[edgao]

/publish connector=connectors/source-postgres-strict-encrypt

:clock2: Publishing the following connectors:
connectors/source-postgres-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/2786007171

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres-strict-encrypt	:x:	:x:

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

[edgao]

can't publish until https://github.com/airbytehq/airbyte/pull/14801 is merged

[edgao]

@yurii-bidiuk I'll take over publishing here.

Will merge https://github.com/airbytehq/airbyte/pull/15187/ into this one, get https://github.com/airbytehq/airbyte/pull/14801 merged to master, and update this PR's version numbers/changelog accordingly.

[edgao]

/publish connector=connectors/source-postgres

:clock2: Publishing the following connectors:
connectors/source-postgres
https://github.com/airbytehq/airbyte/actions/runs/2791193532

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres	:white_check_mark:	:white_check_mark:

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

[edgao]

/publish connector=connectors/source-postgres-strict-encrypt

:clock2: Publishing the following connectors:
connectors/source-postgres-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/2791193613

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres-strict-encrypt	:white_check_mark:	:x:

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️