johnjcasey commented on code in PR #32512:
URL: https://github.com/apache/beam/pull/32512#discussion_r1801226025
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BeamRowToStorageApiProto.java:
##########
@@ -291,9 +307,17 @@ private static Object toProtoValue(
if (arrayElementType == null) {
throw new RuntimeException("Unexpected null element type!");
}
- return list.stream()
- .map(v -> toProtoValue(fieldDescriptor, arrayElementType, v))
- .collect(Collectors.toList());
+ boolean shouldFlatMap =
Review Comment:
Also, can you add a comment to that effect?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BeamRowToStorageApiProto.java:
##########
@@ -243,7 +245,19 @@ private static TableFieldSchema
fieldDescriptorFromBeamField(Field field) {
builder = builder.setType(type);
break;
case MAP:
- throw new RuntimeException("Map types not supported by BigQuery.");
+ @Nullable FieldType keyType = field.getType().getMapKeyType();
+ @Nullable FieldType valueType = field.getType().getMapValueType();
+ if (keyType == null || valueType == null) {
+ throw new RuntimeException("Unexpected null element type!");
Review Comment:
Can you add some context to this exception around the error being in
converting to the storage api proto? That would help users diagnose their
pipelines without needing to know the beam code as well
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryUtils.java:
##########
@@ -403,7 +403,7 @@ private static List<TableFieldSchema>
toTableFieldSchema(Schema schema) {
}
if (type.getTypeName().isCollectionType()) {
type =
Preconditions.checkArgumentNotNull(type.getCollectionElementType());
- if (type.getTypeName().isCollectionType() ||
type.getTypeName().isMapType()) {
+ if (type.getTypeName().isCollectionType() &&
!type.getTypeName().isMapType()) {
Review Comment:
BQ supports arrays of maps, but not arrays of other collections?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BeamRowToStorageApiProto.java:
##########
@@ -291,9 +307,17 @@ private static Object toProtoValue(
if (arrayElementType == null) {
throw new RuntimeException("Unexpected null element type!");
}
- return list.stream()
- .map(v -> toProtoValue(fieldDescriptor, arrayElementType, v))
- .collect(Collectors.toList());
+ boolean shouldFlatMap =
Review Comment:
This looks like it supports one level of nested collection. Is that an
intended limit?
--
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]
