🐛 Source Postgres: write hstore as json #13367

yurii-bidiuk · 2022-06-01T07:10:24Z

What

Fixed #11953

How

Converted hstore key-value to JSON format

🚨 User Impact 🚨

Values of hstore type will be represented as JSON string instead of key-value map

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

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 · 2022-06-01T07:15:13Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2419890300
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2419890300
No Python unittests run

Build Passed

Test summary info:

All Passed

alexandertsukanov · 2022-06-01T07:42:27Z

...postgres/src/main/java/io/airbyte/integrations/source/postgres/PostgresSourceOperations.java

    if (resultSet.getString(colIndex) == null) {
      json.putNull(columnName);
+    } else if (columnTypeName.equalsIgnoreCase("hstore")) {
+      putHstoreAsJson(json, columnName, resultSet, colIndex);
    } else if (columnTypeName.equalsIgnoreCase("bool") || columnTypeName.equalsIgnoreCase("boolean")) {
      putBoolean(json, columnName, resultSet, colIndex);
    } else if (columnTypeName.equalsIgnoreCase("bytea")) {


@yurii-bidiuk refactor this to switch, please. See as example https://github.com/airbytehq/airbyte/pull/13166/files

__
Summary: 🛠️ This code needs a fix | Tags: Inefficient

alexandertsukanov · 2022-06-01T07:43:12Z

...postgres/src/main/java/io/airbyte/integrations/source/postgres/PostgresSourceOperations.java

@@ -169,6 +173,12 @@ private void putMoney(final ObjectNode node, final String columnName, final Resu
    node.put(columnName, DataTypeUtils.returnNullIfInvalid(() -> Double.valueOf(moneyValue), Double::isFinite));
  }

+  private void putHstoreAsJson(final ObjectNode node, final String columnName, final ResultSet resultSet, final int index)
+      throws SQLException {
+    HashMap<String, String> data = (HashMap<String, String>) resultSet.getObject(index);


Are we sure we won't have ClassCastException here?

__
Summary: ❓ I have a question

alexandertsukanov

Left comments, please take a look.

alexandertsukanov

__
Summary: 👌 This code looks good

yurii-bidiuk · 2022-06-02T14:45:42Z

/publish connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2428987691
❌ Failed to publish connectors/source-postgres
❌ Couldn't auto-bump version for connectors/source-postgres

yurii-bidiuk · 2022-06-02T16:13:40Z

@grishick Hi Greg. Publish for this PR is failed, looks like the same issue with timeout as we had before

edgao · 2022-06-02T16:15:49Z

...postgres/src/main/java/io/airbyte/integrations/source/postgres/PostgresSourceOperations.java

+  private void putHstoreAsJson(final ObjectNode node, final String columnName, final ResultSet resultSet, final int index)
+      throws SQLException {
+    final var data = resultSet.getObject(index);
+    node.put(columnName, new Gson().toJson(data));


nitpick: most of our codebase uses ObjectMapper rather than Gson, e.g.

airbyte/airbyte-integrations/connectors/source-sftp/src/main/java/io/airbyte/integrations/source/sftp/parsers/CsvFileParser.java

Line 20 in da88158

private final ObjectMapper objectMapper = MoreMappers.initMapper();

I think it would be better to declare the object mapper as a static final constant

edgao · 2022-06-02T16:16:39Z

...airbyte/integrations/io/airbyte/integration_tests/sources/CdcPostgresSourceDatatypeTest.java

+        TestDataHolder.builder()
+            .sourceType("hstore")
+            .airbyteType(JsonSchemaType.STRING)
+            .addInsertValues("'\"paperback\" => \"243\",\"publisher\" => \"postgresqltutorial.com\","


nitpick: these would be a bit more readable using multiline strings:

.addInsertValues(""" "paperback" => ... """)

could you also do this for the json blob on the next line? https://github.com/airbytehq/airbyte/pull/13367/files#diff-a2069083fdac6437ab9a15a5376aeccbb550b9a05a7c28ea483bd4ea8bff1437R555

yurii-bidiuk · 2022-06-06T11:19:45Z

/publish connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2447396807
🚀 Successfully published connectors/source-postgres
🚀 Auto-bumped version for connectors/source-postgres
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2447396807

github-actions bot added the area/connectors Connector related issues label Jun 1, 2022

yurii-bidiuk force-pushed the bidiuk/11953-src-postgres-hstore-support branch from 4c60f62 to 51cf252 Compare June 1, 2022 07:14

yurii-bidiuk mentioned this pull request Jun 1, 2022

Different serialization behaviour for Postgres hstore type between normal and CDC sync #11953

Closed

yurii-bidiuk requested review from alexandertsukanov and andriikorotkov June 1, 2022 07:18

alexandertsukanov reviewed Jun 1, 2022

View reviewed changes

alexandertsukanov suggested changes Jun 1, 2022

View reviewed changes

yurii-bidiuk force-pushed the bidiuk/11953-src-postgres-hstore-support branch 2 times, most recently from 6e5f433 to e24565f Compare June 1, 2022 08:02

andriikorotkov approved these changes Jun 1, 2022

View reviewed changes

alexandertsukanov approved these changes Jun 1, 2022

View reviewed changes

yurii-bidiuk requested a review from edgao June 1, 2022 12:29

grishick approved these changes Jun 1, 2022

View reviewed changes

github-actions bot added the area/documentation Improvements or additions to documentation label Jun 2, 2022

yurii-bidiuk force-pushed the bidiuk/11953-src-postgres-hstore-support branch from ecb87b6 to 9b6e2b3 Compare June 2, 2022 07:35

yurii-bidiuk temporarily deployed to more-secrets June 2, 2022 07:37 Inactive

alexandr-shegeda marked this pull request as ready for review June 2, 2022 09:52

yurii-bidiuk force-pushed the bidiuk/11953-src-postgres-hstore-support branch from 9b6e2b3 to 2ea9d94 Compare June 2, 2022 14:33

yurii-bidiuk temporarily deployed to more-secrets June 2, 2022 14:36 Inactive

edgao approved these changes Jun 2, 2022

View reviewed changes

bleonard added the connectors/source/postgres label Jun 2, 2022

yurii-bidiuk force-pushed the bidiuk/11953-src-postgres-hstore-support branch from 2ea9d94 to 8b23971 Compare June 3, 2022 08:48

yurii-bidiuk temporarily deployed to more-secrets June 3, 2022 08:50 Inactive

yurii-bidiuk force-pushed the bidiuk/11953-src-postgres-hstore-support branch from 8b23971 to ce9c9c0 Compare June 6, 2022 10:33

yurii-bidiuk temporarily deployed to more-secrets June 6, 2022 10:36 Inactive

yurii-bidiuk force-pushed the bidiuk/11953-src-postgres-hstore-support branch from ce9c9c0 to 918eb71 Compare June 6, 2022 11:17

yurii-bidiuk force-pushed the bidiuk/11953-src-postgres-hstore-support branch from 918eb71 to 75c3bdf Compare June 6, 2022 11:18

octavia-squidington-iii temporarily deployed to more-secrets June 6, 2022 13:01 Inactive

yurii-bidiuk added 5 commits June 6, 2022 17:41

src-postgres: write hstore as json

9ce6d1d

fix PR remarks

10f9c8d

bump version

497bdb5

fix PR comments

731cbf8

add text blocks

1b58cd5

yurii-bidiuk force-pushed the bidiuk/11953-src-postgres-hstore-support branch from 3ec4441 to 1b58cd5 Compare June 6, 2022 14:42

yurii-bidiuk merged commit 4b266c1 into master Jun 6, 2022

yurii-bidiuk deleted the bidiuk/11953-src-postgres-hstore-support branch June 6, 2022 19:25

octavia-squidington-iii mentioned this pull request Jun 6, 2022

Bump Airbyte version from 0.39.11-alpha to 0.39.12-alpha #13512

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Source Postgres: write hstore as json #13367

🐛 Source Postgres: write hstore as json #13367

yurii-bidiuk commented Jun 1, 2022

yurii-bidiuk commented Jun 1, 2022 •

edited by github-actions bot

Loading

alexandertsukanov Jun 1, 2022

yurii-bidiuk Jun 1, 2022

alexandertsukanov Jun 1, 2022

yurii-bidiuk Jun 1, 2022

alexandertsukanov left a comment

alexandertsukanov left a comment

yurii-bidiuk commented Jun 2, 2022 •

edited by github-actions bot

Loading

yurii-bidiuk commented Jun 2, 2022

edgao Jun 2, 2022

yurii-bidiuk Jun 3, 2022

edgao Jun 3, 2022

yurii-bidiuk Jun 6, 2022

edgao Jun 2, 2022

yurii-bidiuk Jun 3, 2022

edgao Jun 3, 2022

yurii-bidiuk Jun 6, 2022

yurii-bidiuk commented Jun 6, 2022 •

edited by github-actions bot

Loading

🐛 Source Postgres: write hstore as json #13367

🐛 Source Postgres: write hstore as json #13367

Conversation

yurii-bidiuk commented Jun 1, 2022

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

yurii-bidiuk commented Jun 1, 2022 • edited by github-actions bot Loading

Build Passed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexandertsukanov left a comment

Choose a reason for hiding this comment

alexandertsukanov left a comment

Choose a reason for hiding this comment

yurii-bidiuk commented Jun 2, 2022 • edited by github-actions bot Loading

yurii-bidiuk commented Jun 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yurii-bidiuk commented Jun 6, 2022 • edited by github-actions bot Loading

yurii-bidiuk commented Jun 1, 2022 •

edited by github-actions bot

Loading

yurii-bidiuk commented Jun 2, 2022 •

edited by github-actions bot

Loading

yurii-bidiuk commented Jun 6, 2022 •

edited by github-actions bot

Loading