You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/10/13 09:05:10 UTC
[GitHub] [beam] kkdoon opened a new pull request, #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
kkdoon opened a new pull request, #23620:
URL: https://github.com/apache/beam/pull/23620
As discussed in issue #23541, `BigQueryAvroUtils` does not correctly generate the Avro schema for certain BigQuery data types, such as `Numeric` and `BigNumeric`. This PR address this issue and generates the correct avro schema. This further helps in transformation of GenericRecord -> TableRow, via `convertGenericRecordToTableRow` function, that rely on these BigQuery data types.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [x] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/get-started-contributing/#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #23620:
URL: https://github.com/apache/beam/pull/23620#issuecomment-1307691390
@ahmedabu98 what are next steps here? Looks like we may just need a committer review - @apilloud could you take a look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
apilloud commented on PR #23620:
URL: https://github.com/apache/beam/pull/23620#issuecomment-1293791489
R: @steveniemitz @kennknowles @vachan-shetty
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kkdoon commented on a diff in pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
kkdoon commented on code in PR #23620:
URL: https://github.com/apache/beam/pull/23620#discussion_r1014496108
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIOReadTest.java:
##########
@@ -403,11 +483,21 @@ private void testReadFromTable(boolean useTemplateCompatibility, boolean useRead
fakeDatasetService.createDataset("non-executing-project", "somedataset", "", "", null);
fakeDatasetService.createTable(sometable);
+ byte[] bytes = new BigDecimal("10.223").unscaledValue().toByteArray();
List<TableRow> records =
Lists.newArrayList(
- new TableRow().set("name", "a").set("number", 1L),
- new TableRow().set("name", "b").set("number", 2L),
- new TableRow().set("name", "c").set("number", 3L));
+ new TableRow()
+ .set("name", "a")
+ .set("number", 1L)
+ .set("numeric_num", ByteBuffer.wrap(bytes)),
+ new TableRow()
+ .set("name", "b")
+ .set("number", 2L)
+ .set("numeric_num", ByteBuffer.wrap(bytes)),
+ new TableRow()
+ .set("name", "c")
+ .set("number", 3L)
+ .set("numeric_num", ByteBuffer.wrap(bytes)));
Review Comment:
sounds good, i have updated the this test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #23620:
URL: https://github.com/apache/beam/pull/23620#issuecomment-1277333378
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud merged pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
apilloud merged PR #23620:
URL: https://github.com/apache/beam/pull/23620
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ahmedabu98 commented on a diff in pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
ahmedabu98 commented on code in PR #23620:
URL: https://github.com/apache/beam/pull/23620#discussion_r1007086324
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIOReadTest.java:
##########
@@ -403,11 +483,21 @@ private void testReadFromTable(boolean useTemplateCompatibility, boolean useRead
fakeDatasetService.createDataset("non-executing-project", "somedataset", "", "", null);
fakeDatasetService.createTable(sometable);
+ byte[] bytes = new BigDecimal("10.223").unscaledValue().toByteArray();
List<TableRow> records =
Lists.newArrayList(
- new TableRow().set("name", "a").set("number", 1L),
- new TableRow().set("name", "b").set("number", 2L),
- new TableRow().set("name", "c").set("number", 3L));
+ new TableRow()
+ .set("name", "a")
+ .set("number", 1L)
+ .set("numeric_num", ByteBuffer.wrap(bytes)),
+ new TableRow()
+ .set("name", "b")
+ .set("number", 2L)
+ .set("numeric_num", ByteBuffer.wrap(bytes)),
+ new TableRow()
+ .set("name", "c")
+ .set("number", 3L)
+ .set("numeric_num", ByteBuffer.wrap(bytes)));
Review Comment:
The read pipeline in this test still only looks for the `name` and `number` fields. Can you adjust this so that it also looks for and tests the numeric type?
P.S. if you add another numeric field you can use your MyData class instead of KVs here as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kkdoon commented on pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
kkdoon commented on PR #23620:
URL: https://github.com/apache/beam/pull/23620#issuecomment-1293085586
assign set of reviewers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a diff in pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
apilloud commented on code in PR #23620:
URL: https://github.com/apache/beam/pull/23620#discussion_r1017317167
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryAvroUtils.java:
##########
@@ -448,4 +449,32 @@ private static Field convertField(TableFieldSchema bigQueryField) {
bigQueryField.getDescription(),
(Object) null /* Cast to avoid deprecated JsonNode constructor. */);
}
+
+ private static Schema handleAvroLogicalTypes(TableFieldSchema bigQueryField, Type avroType) {
+ String bqType = bigQueryField.getType();
+ switch (bqType) {
+ case "NUMERIC":
+ // Default value based on
+ // https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#decimal_types
+ int precision = Optional.ofNullable(bigQueryField.getPrecision()).orElse(38L).intValue();
+ int scale = Optional.ofNullable(bigQueryField.getScale()).orElse(9L).intValue();
+ return LogicalTypes.decimal(precision, scale).addToSchema(Schema.create(Type.BYTES));
+ case "BIGNUMERIC":
+ // Default value based on
+ // https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#decimal_types
+ int precisionBigNumeric =
+ Optional.ofNullable(bigQueryField.getPrecision()).orElse(77L).intValue();
Review Comment:
The tests you describe don't seem to be in the code and this didn't answer my question but I guess the answer is in the initial question. A value of `77` will at least pull the failure into something the user can fix with additional constraints within their pipeline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kkdoon commented on a diff in pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
kkdoon commented on code in PR #23620:
URL: https://github.com/apache/beam/pull/23620#discussion_r1017394461
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryAvroUtils.java:
##########
@@ -448,4 +449,32 @@ private static Field convertField(TableFieldSchema bigQueryField) {
bigQueryField.getDescription(),
(Object) null /* Cast to avoid deprecated JsonNode constructor. */);
}
+
+ private static Schema handleAvroLogicalTypes(TableFieldSchema bigQueryField, Type avroType) {
+ String bqType = bigQueryField.getType();
+ switch (bqType) {
+ case "NUMERIC":
+ // Default value based on
+ // https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#decimal_types
+ int precision = Optional.ofNullable(bigQueryField.getPrecision()).orElse(38L).intValue();
+ int scale = Optional.ofNullable(bigQueryField.getScale()).orElse(9L).intValue();
+ return LogicalTypes.decimal(precision, scale).addToSchema(Schema.create(Type.BYTES));
+ case "BIGNUMERIC":
+ // Default value based on
+ // https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#decimal_types
+ int precisionBigNumeric =
+ Optional.ofNullable(bigQueryField.getPrecision()).orElse(77L).intValue();
Review Comment:
sorry, i was not referring to these unit tests. I tested with a standalone beam job, in direct runner mode, against an actual BigQuery table and verified the default precision/scale value set in the Avro writer schema (that's why chose to set default as 77 instead of 76). Also, tried inserting and reading the max documented value for BigNumeric field and it works for numbers with 77 precision. I could add a BQ integration test to test this edge case, if that helps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #23620:
URL: https://github.com/apache/beam/pull/23620#issuecomment-1293086536
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @apilloud for label java.
R: @ahmedabu98 for label io.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kkdoon commented on a diff in pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
kkdoon commented on code in PR #23620:
URL: https://github.com/apache/beam/pull/23620#discussion_r1017277186
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryAvroUtils.java:
##########
@@ -448,4 +449,32 @@ private static Field convertField(TableFieldSchema bigQueryField) {
bigQueryField.getDescription(),
(Object) null /* Cast to avoid deprecated JsonNode constructor. */);
}
+
+ private static Schema handleAvroLogicalTypes(TableFieldSchema bigQueryField, Type avroType) {
+ String bqType = bigQueryField.getType();
+ switch (bqType) {
+ case "NUMERIC":
+ // Default value based on
+ // https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#decimal_types
+ int precision = Optional.ofNullable(bigQueryField.getPrecision()).orElse(38L).intValue();
+ int scale = Optional.ofNullable(bigQueryField.getScale()).orElse(9L).intValue();
+ return LogicalTypes.decimal(precision, scale).addToSchema(Schema.create(Type.BYTES));
+ case "BIGNUMERIC":
+ // Default value based on
+ // https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#decimal_types
+ int precisionBigNumeric =
+ Optional.ofNullable(bigQueryField.getPrecision()).orElse(77L).intValue();
Review Comment:
I ran a beam job and tried reading from an actual BigQuery table with BIGNUMERIC field. The [writer schema](https://github.com/apache/beam/blob/483a0c95734c528aa45419596e9f27e9e650c5d7/sdks/java/core/src/main/java/org/apache/beam/sdk/io/AvroSource.java#L754), set in the extracted Avro file, has precision value set as 77 and the scale is set to 38, for fields that have no explicit precision/scale specified.
I also tried inserting the max supported value that has a precision of 77 and it works as long as its <= 578960446186580977117854925043439539266.34992332820282019728792003956564819967 (since 77th digit is partial it supports this value and not 999999999999999999999999999999999999999.99999999999999999999999999999999999999 instead)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #23620:
URL: https://github.com/apache/beam/pull/23620#issuecomment-1293908347
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a diff in pull request #23620: Handle Avro schema generation for logical data types in BigQueryAvroUtils
Posted by GitBox <gi...@apache.org>.
apilloud commented on code in PR #23620:
URL: https://github.com/apache/beam/pull/23620#discussion_r1017037762
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryAvroUtils.java:
##########
@@ -448,4 +449,32 @@ private static Field convertField(TableFieldSchema bigQueryField) {
bigQueryField.getDescription(),
(Object) null /* Cast to avoid deprecated JsonNode constructor. */);
}
+
+ private static Schema handleAvroLogicalTypes(TableFieldSchema bigQueryField, Type avroType) {
+ String bqType = bigQueryField.getType();
+ switch (bqType) {
+ case "NUMERIC":
+ // Default value based on
+ // https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#decimal_types
+ int precision = Optional.ofNullable(bigQueryField.getPrecision()).orElse(38L).intValue();
+ int scale = Optional.ofNullable(bigQueryField.getScale()).orElse(9L).intValue();
+ return LogicalTypes.decimal(precision, scale).addToSchema(Schema.create(Type.BYTES));
+ case "BIGNUMERIC":
+ // Default value based on
+ // https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#decimal_types
+ int precisionBigNumeric =
+ Optional.ofNullable(bigQueryField.getPrecision()).orElse(77L).intValue();
Review Comment:
The documentation you linked says `76.76` and the API docs say `1 <= precision - scale <= 38 and 0 <= scale <= 38` which means the max value for precision is `76`. I'm not sure what will work in practice it seems like `76` would potentially fail to read from BigQuery and `77` will fail to write to BigQuery. Can you provide any more info on why 77 was chosen? Should there be tests for this edge case?
https://cloud.google.com/java/docs/reference/google-cloud-bigquerystorage/2.8.2/com.google.cloud.bigquery.storage.v1.TableFieldSchema.Builder#com_google_cloud_bigquery_storage_v1_TableFieldSchema_Builder_getPrecision__
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org