-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[java] BQ: use logical type in avro schema factory on write #33163
Conversation
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
ffaf4dc
to
fe9233e
Compare
Assigning reviewers. If you would like to opt out of this review, comment R: @Abacn for label java. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Reminder, please take a look at this pr: @Abacn @damondouglas |
@@ -3625,7 +3622,8 @@ private <DestinationT> WriteResult expandTyped( | |||
hasSchema, | |||
"A schema must be provided if an avroFormatFunction " | |||
+ "is set but no avroSchemaFactory is defined."); | |||
avroSchemaFactory = DEFAULT_AVRO_SCHEMA_FACTORY; | |||
avroSchemaFactory = | |||
(tableSchema) -> BigQueryUtils.toGenericAvroSchema(tableSchema, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aren't these no-op change?
BigQueryAvroUtils.toGenericAvroSchema(schema) calls BigQueryAvroUtils.toGenericAvroSchema(schema, true) calls BigQueryAvroUtils.toGenericAvroSchema("root", schema, true, namespace)
BigQueryUtils.toGenericAvroSchema(schema, true) calls BigQueryAvroUtils.toGenericAvroSchema("root", schema, true, namespace)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I messed up with BigQueryUtils.toGenericAvroSchema
which has opposite default
when writing to BQ with avro, if the table schema contains
DATE
,TIME
,TIMESTAMP
columns, the default schema factory should create avro fields with matching logical type.There is still an issue with
DATETIME
:BigQueryAvroUtils::toGenericAvroSchema
favors generating schema for the reading side and generates an avro field withstring(datetime)
type. This can't be used on write (expectinglong(local-timestamp-millis)
orlong(local-timestamp-micros)
).See note in doc