reuvenlax commented on code in PR #29114:
URL: https://github.com/apache/beam/pull/29114#discussion_r1370810930
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProto.java:
##########
@@ -213,10 +213,15 @@ private static String
getPrettyFieldName(SchemaInformation schema) {
.put(TableFieldSchema.Type.JSON, "JSON")
.build();
- public static TableFieldSchema.Mode modeToProtoMode(String mode) {
+ public static TableFieldSchema.Mode modeToProtoMode(
+ @Nullable String defaultValueExpression, String mode) {
+ // If there is a default value expression, treat this field as if it were
nullable.
+ if (defaultValueExpression != null) {
+ return TableFieldSchema.Mode.NULLABLE;
+ }
Review Comment:
this is a bit weird here:
- REQUIRED is basically a noop if taking a default value, as the value
will never be missing.
- However if defaulting to NULL, then REQUIRED still has meaning.
- Also users can explicitly pass in NULL for field values (overriding the
default value). In this case, REQUIRED prevents the user from doing this (I
think).
The second and third bullet points aren't properly handled by this code, as
I'm not entirely sure how to express that in the proto schema being generated
here. I think this validation will still happen on the BQ side, so failing rows
will still end up in the failedRows PCollection. We should probably double
check the expected semantics with the BQ team though.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
