robinyqiu commented on a change in pull request #13588:
URL: https://github.com/apache/beam/pull/13588#discussion_r550321908
##########
File path:
sdks/java/extensions/sql/datacatalog/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/datacatalog/SchemaUtils.java
##########
@@ -128,18 +128,15 @@ private static ColumnSchema fromBeamField(Schema.Field
field) {
}
ColumnSchema column =
fromBeamField(Field.of(field.getName(),
fieldType.getCollectionElementType()));
- if (!column.getMode().isEmpty()) {
Review comment:
Done.
I think this check is redundant for now because the only place that sets
mode is the else branch below that the array element check will surely enter,
because we already checked array element is not ARRAY type. And we have also
checked the array element is not nullable above so it is always `REQUIRED`. But
I agree leaving this check here can prevent code change that breaks this in the
future.
##########
File path:
sdks/java/extensions/sql/datacatalog/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/datacatalog/DataCatalogTableProvider.java
##########
@@ -178,6 +214,23 @@ private Table toCalciteTable(String tableName, Entry
entry) {
return tableBuilder.get().schema(schema).name(tableName).build();
}
+ public boolean setSchemaIfNotPresent(String resource, Schema schema) {
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
