ahmedabu98 commented on code in PR #36509:
URL: https://github.com/apache/beam/pull/36509#discussion_r2603570230


##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/parser/SqlCreateExternalTable.java:
##########
@@ -159,7 +159,10 @@ public void execute(CalcitePrepare.Context context) {
       CatalogManagerSchema catalogManagerSchema = (CatalogManagerSchema) 
schema;
       catalogManagerSchema.maybeRegisterProvider(pathOverride, 
SqlDdlNodes.getString(type));
 
-      CatalogSchema catalogSchema = 
catalogManagerSchema.getCatalogSchema(pathOverride);
+      CatalogSchema catalogSchema =
+          pathOverride.catalog() != null

Review Comment:
   I put the check inside `catalogManagerSchema.getCatalogSchema(TableName)` to 
make it cleaner. Now it returns either the specified catalog schema or the 
default if no catalog is specified. Should work for current use cases



##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/parser/SqlDropTable.java:
##########
@@ -53,7 +53,10 @@ public void execute(CalcitePrepare.Context context) {
 
     BeamCalciteSchema beamCalciteSchema;
     if (schema instanceof CatalogManagerSchema) {
-      CatalogSchema catalogSchema = ((CatalogManagerSchema) 
schema).getCatalogSchema(pathOverride);
+      CatalogSchema catalogSchema =
+          pathOverride.catalog() != null

Review Comment:
   > But we should probably be able to make a common "catalog reference" and 
"table reference" construct.
   
   Not sure I understand this, but lmk if the new change takes care of it



-- 
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]

Reply via email to