Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 source-postgres: fix jsonb[] handling bug #30534

Merged
merged 3 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ ENV APPLICATION source-postgres

COPY --from=build /airbyte /airbyte

LABEL io.airbyte.version=3.1.8
LABEL io.airbyte.version=3.1.9
LABEL io.airbyte.name=airbyte/source-postgres
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ data:
connectorSubtype: database
connectorType: source
definitionId: decd338e-5647-4c0b-adf4-da0e75f5a750
dockerImageTag: 3.1.8
dockerImageTag: 3.1.9
maxSecondsBetweenMessages: 7200
dockerRepository: airbyte/source-postgres
githubIssueLabel: source-postgres
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,13 @@ public void copyToJsonField(final ResultSet resultSet, final int colIndex, final
case TIMESTAMP -> putTimestamp(json, columnName, resultSet, colIndex);
case BLOB, BINARY, VARBINARY, LONGVARBINARY -> putBinary(json, columnName, resultSet, colIndex);
case ARRAY -> putArray(json, columnName, resultSet, colIndex);
default -> json.put(columnName, value);
default -> {
if (columnInfo.columnType.isArrayType()) {
putArray(json, columnName, resultSet, colIndex);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come jsonb[] type not falling into case ARRAY ?

} else {
json.put(columnName, value);
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,15 @@ public Integer getVendorTypeNumber() {
return type;
}

/**
* Returns true if the PostgresType is an array type, false otherwise.
*
* @return true if the PostgresType is an array type, false otherwise.
*/
public boolean isArrayType() {
return type == Types.ARRAY;
}

/**
* Returns the {@code JDBCType} that corresponds to the specified {@code Types} value
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,13 +283,13 @@ public PreparedStatement createCtidQueryStatement(final Connection connection,
final PreparedStatement preparedStatement = connection.prepareStatement(sql);
preparedStatement.setObject(1, lowerBound.toString());
preparedStatement.setObject(2, upperBound.toString());
LOGGER.info("Executing query for table {}: {}", tableName, preparedStatement);
LOGGER.info("Executing query for table {}: {} with bindings {} and {}", tableName, sql, lowerBound, upperBound);
return preparedStatement;
} else {
final String sql = "SELECT ctid::text, %s FROM %s WHERE ctid > ?::tid".formatted(wrappedColumnNames, fullTableName);
final PreparedStatement preparedStatement = connection.prepareStatement(sql);
preparedStatement.setObject(1, lowerBound.toString());
LOGGER.info("Executing query for table {}: {}", tableName, preparedStatement);
LOGGER.info("Executing query for table {}: {} with binding {}", tableName, sql, lowerBound);
return preparedStatement;
}
} catch (final SQLException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,17 @@ private void addArraysTestData() {
.addExpectedValues("[131070.237689,231072.476596593]")
.build());

addDataTypeTestData(
TestDataHolder.builder()
.sourceType("jsonb_array")
.fullSourceDataType("JSONB[]")
.airbyteType(JsonSchemaType.builder(JsonSchemaPrimitive.ARRAY)
.withItems(JsonSchemaType.builder(JsonSchemaPrimitive.STRING).build())
.build())
.addInsertValues("ARRAY['{\"foo\":\"bar\"}'::JSONB, NULL]")
.addExpectedValues("[\"{\\\"foo\\\": \\\"bar\\\"}\",null]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still seem likes Postgres's string representation of the array. I think we want something along the lines of ["{\"foo\": \"bar\"}"]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, disregard this. It's Java's representation of the array. We've tested this version manually and it fixes the regression 👍

.build());

addDataTypeTestData(
TestDataHolder.builder()
.sourceType("money_array")
Expand Down
1 change: 1 addition & 0 deletions docs/integrations/sources/postgres.md
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ According to Postgres [documentation](https://www.postgresql.org/docs/14/datatyp

| Version | Date | Pull Request | Subject |
|---------|------------|----------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| 3.1.9 | 2023-09-25 | [30534](https://github.com/airbytehq/airbyte/pull/30534) | Fix JSONB[] column type handling bug. |
| 3.1.8 | 2023-09-20 | [30125](https://github.com/airbytehq/airbyte/pull/30125) | Improve initial load performance for older versions of PostgreSQL. |
| 3.1.7 | 2023-09-05 | [29672](https://github.com/airbytehq/airbyte/pull/29672) | Handle VACUUM happening during initial sync |
| 3.1.6 | 2023-08-24 | [29821](https://github.com/airbytehq/airbyte/pull/29821) | Set replication_method display_type to radio, update titles and descriptions, and make CDC the default choice |
Expand Down
Loading