From 342c290825fc212a2eeb03916995c8d6aaf9b86d Mon Sep 17 00:00:00 2001 From: snuyanzin Date: Mon, 19 Aug 2024 15:15:46 +0200 Subject: [PATCH] Address feedback --- .../parse/HiveParserDDLSemanticAnalyzer.java | 15 ++++++++---- .../operations/AbstractShowOperation.java | 6 ++--- .../operations/ShowDatabasesOperation.java | 13 ++++------ .../operations/ShowFunctionsOperation.java | 18 ++++++++------ .../operations/ShowProceduresOperation.java | 24 +++++++------------ .../table/operations/ShowTablesOperation.java | 24 +++++++++---------- .../table/operations/ShowViewsOperation.java | 24 +++++++++---------- .../converters/AbstractSqlShowConverter.java | 17 +++++++++---- .../converters/SqlShowDatabasesConverter.java | 6 ++++- .../converters/SqlShowFunctionsConverter.java | 8 +++++-- .../converters/SqlShowProcedureConverter.java | 7 ++++-- .../converters/SqlShowTablesConverter.java | 8 +++++-- .../converters/SqlShowViewsConverter.java | 8 +++++-- 13 files changed, 102 insertions(+), 76 deletions(-) diff --git a/flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/parse/HiveParserDDLSemanticAnalyzer.java b/flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/parse/HiveParserDDLSemanticAnalyzer.java index 8b72fece0a2aa..bd8ef491519da 100644 --- a/flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/parse/HiveParserDDLSemanticAnalyzer.java +++ b/flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/parse/HiveParserDDLSemanticAnalyzer.java @@ -321,7 +321,7 @@ public Operation convertToOperation(HiveParserASTNode ast) throws SemanticExcept res = convertDescribeTable(ast); break; case HiveASTParser.TOK_SHOWDATABASES: - res = convertShowDatabases(); + res = convertShowDatabases(catalogRegistry.getCurrentCatalog()); break; case HiveASTParser.TOK_SHOWTABLES: res = convertShowTables(ast, false); @@ -1803,8 +1803,8 @@ private Operation convertShowPartitions(HiveParserASTNode ast) throws SemanticEx HiveConf.getVar(conf, HiveConf.ConfVars.DEFAULTPARTITIONNAME)); } - private Operation convertShowDatabases() { - return new ShowDatabasesOperation(); + private Operation convertShowDatabases(String catalogName) { + return new ShowDatabasesOperation(catalogName); } private Operation convertShowTables(HiveParserASTNode ast, boolean expectView) { @@ -1843,7 +1843,11 @@ private Operation convertShowTables(HiveParserASTNode ast, boolean expectView) { if (pattern != null) { handleUnsupportedOperation("SHOW TABLES/VIEWS LIKE is not supported"); } - return expectView ? new ShowViewsOperation() : new ShowTablesOperation(); + return expectView + ? new ShowViewsOperation( + catalogRegistry.getCurrentCatalog(), catalogRegistry.getCurrentDatabase()) + : new ShowTablesOperation( + catalogRegistry.getCurrentCatalog(), catalogRegistry.getCurrentDatabase()); } /** @@ -1857,7 +1861,8 @@ private Operation convertShowFunctions(HiveParserASTNode ast) { assert (ast.getChild(0).getType() == HiveASTParser.KW_LIKE); throw new ValidationException("SHOW FUNCTIONS LIKE is not supported yet"); } - return new ShowFunctionsOperation(); + return new ShowFunctionsOperation( + catalogRegistry.getCurrentCatalog(), catalogRegistry.getCurrentDatabase()); } private Operation convertAlterTableRename( diff --git a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/AbstractShowOperation.java b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/AbstractShowOperation.java index 08d2bff47db88..9a4984e317ebb 100644 --- a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/AbstractShowOperation.java +++ b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/AbstractShowOperation.java @@ -38,14 +38,12 @@ */ @Internal public abstract class AbstractShowOperation implements ShowOperation { - protected final @Nullable String catalogName; + protected final String catalogName; protected final @Nullable String preposition; protected final @Nullable ShowLikeOperator likeOp; public AbstractShowOperation( - @Nullable String catalogName, - @Nullable String preposition, - @Nullable ShowLikeOperator likeOp) { + String catalogName, @Nullable String preposition, @Nullable ShowLikeOperator likeOp) { this.catalogName = catalogName; this.preposition = preposition; this.likeOp = likeOp; diff --git a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowDatabasesOperation.java b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowDatabasesOperation.java index e610465209582..80bbf6cb774be 100644 --- a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowDatabasesOperation.java +++ b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowDatabasesOperation.java @@ -38,24 +38,21 @@ public class ShowDatabasesOperation extends AbstractShowOperation { public ShowDatabasesOperation( - @Nullable String catalogName, - @Nullable String preposition, - @Nullable ShowLikeOperator likeOp) { + String catalogName, @Nullable String preposition, @Nullable ShowLikeOperator likeOp) { super(catalogName, preposition, likeOp); } - public ShowDatabasesOperation(ShowLikeOperator likeOp) { - this(null, null, likeOp); + public ShowDatabasesOperation(String catalogName, ShowLikeOperator likeOp) { + this(catalogName, null, likeOp); } - public ShowDatabasesOperation() { - this(null, null, null); + public ShowDatabasesOperation(String catalogName) { + this(catalogName, null, null); } @Override protected Collection retrieveDataForTableResult(Context ctx) { final CatalogManager catalogManager = ctx.getCatalogManager(); - final String catalogName = catalogManager.qualifyCatalog(this.catalogName); return catalogManager.getCatalogOrThrowException(catalogName).listDatabases(); } diff --git a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowFunctionsOperation.java b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowFunctionsOperation.java index 631ef02427d74..dab3f12def0b0 100644 --- a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowFunctionsOperation.java +++ b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowFunctionsOperation.java @@ -55,22 +55,26 @@ public enum FunctionScope { } private final FunctionScope functionScope; - private final @Nullable String databaseName; + private final String databaseName; - public ShowFunctionsOperation() { + public ShowFunctionsOperation(String catalogName, String databaseName) { // "SHOW FUNCTIONS" default is ALL scope - this(FunctionScope.ALL, null); + this(FunctionScope.ALL, catalogName, databaseName, null); } - public ShowFunctionsOperation(FunctionScope functionScope, @Nullable ShowLikeOperator likeOp) { - this(functionScope, null, null, null, likeOp); + public ShowFunctionsOperation( + FunctionScope functionScope, + String catalogName, + String databaseName, + @Nullable ShowLikeOperator likeOp) { + this(functionScope, null, catalogName, databaseName, likeOp); } public ShowFunctionsOperation( FunctionScope functionScope, @Nullable String preposition, - @Nullable String catalogName, - @Nullable String databaseName, + String catalogName, + String databaseName, @Nullable ShowLikeOperator likeOp) { super(catalogName, preposition, likeOp); this.functionScope = functionScope; diff --git a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowProceduresOperation.java b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowProceduresOperation.java index 29d259090da8b..56d56d1f45b95 100644 --- a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowProceduresOperation.java +++ b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowProceduresOperation.java @@ -41,44 +41,38 @@ @Internal public class ShowProceduresOperation extends AbstractShowOperation { - private final @Nullable String databaseName; + private final String databaseName; public ShowProceduresOperation( - @Nullable String catalogName, - @Nullable String databaseName, + String catalogName, + String databaseName, @Nullable String preposition, @Nullable ShowLikeOperator likeOp) { super(catalogName, preposition, likeOp); this.databaseName = databaseName; } - public ShowProceduresOperation(@Nullable ShowLikeOperator likeOp) { - this(null, null, null, likeOp); + public ShowProceduresOperation( + String catalogName, String databaseName, @Nullable ShowLikeOperator likeOp) { + this(catalogName, databaseName, null, likeOp); } @Override protected Collection retrieveDataForTableResult(Context ctx) { final CatalogManager catalogManager = ctx.getCatalogManager(); - final String catalogName = catalogManager.qualifyCatalog(this.catalogName); - final String dbName = catalogManager.qualifyDatabase(this.databaseName); try { if (preposition == null) { // it's to show current_catalog.current_database - return catalogManager - .getCatalogOrError(catalogManager.getCurrentCatalog()) - .listProcedures(catalogManager.getCurrentDatabase()); + return catalogManager.getCatalogOrError(catalogName).listProcedures(databaseName); } else { Catalog catalog = catalogManager.getCatalogOrThrowException(catalogName); - return catalog.listProcedures(dbName); + return catalog.listProcedures(databaseName); } } catch (DatabaseNotExistException e) { throw new TableException( String.format( "Fail to show procedures because the Database `%s` to show from/in does not exist in Catalog `%s`.", - preposition == null ? catalogManager.getCurrentDatabase() : dbName, - preposition == null - ? catalogManager.getCurrentCatalog() - : catalogName)); + databaseName, catalogName)); } } diff --git a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowTablesOperation.java b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowTablesOperation.java index aa05baa689772..348c9df6c2112 100644 --- a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowTablesOperation.java +++ b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowTablesOperation.java @@ -39,39 +39,39 @@ @Internal public class ShowTablesOperation extends AbstractShowOperation { - private final @Nullable String databaseName; + private final String databaseName; public ShowTablesOperation( - @Nullable String catalogName, - @Nullable String databaseName, + String catalogName, + String databaseName, @Nullable String preposition, @Nullable ShowLikeOperator likeOp) { super(catalogName, preposition, likeOp); this.databaseName = databaseName; } - public ShowTablesOperation(@Nullable ShowLikeOperator likeOp) { - this(null, null, null, likeOp); + public ShowTablesOperation( + String catalogName, String databaseName, @Nullable ShowLikeOperator likeOp) { + this(catalogName, databaseName, null, likeOp); } - public ShowTablesOperation() { - this(null); + public ShowTablesOperation(String catalogName, String databaseName) { + this(catalogName, databaseName, null); } @Override protected Set retrieveDataForTableResult(Context ctx) { final CatalogManager catalogManager = ctx.getCatalogManager(); - final String catalogName = catalogManager.qualifyCatalog(this.catalogName); - final String dbName = catalogManager.qualifyDatabase(this.databaseName); if (preposition == null) { return catalogManager.listTables(); } else { Catalog catalog = catalogManager.getCatalogOrThrowException(catalogName); - if (catalog.databaseExists(dbName)) { - return catalogManager.listTables(catalogName, dbName); + if (catalog.databaseExists(databaseName)) { + return catalogManager.listTables(catalogName, databaseName); } else { throw new ValidationException( - String.format("Database '%s'.'%s' doesn't exist.", catalogName, dbName)); + String.format( + "Database '%s'.'%s' doesn't exist.", catalogName, databaseName)); } } } diff --git a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowViewsOperation.java b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowViewsOperation.java index 695f9f8876ad8..180cea511e7d9 100644 --- a/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowViewsOperation.java +++ b/flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ShowViewsOperation.java @@ -39,23 +39,24 @@ @Internal public class ShowViewsOperation extends AbstractShowOperation { - private final @Nullable String databaseName; + private final String databaseName; public ShowViewsOperation( - @Nullable String catalogName, - @Nullable String databaseName, + String catalogName, + String databaseName, @Nullable String preposition, @Nullable ShowLikeOperator likeOp) { super(catalogName, preposition, likeOp); this.databaseName = databaseName; } - public ShowViewsOperation(@Nullable ShowLikeOperator likeOp) { - this(null, null, null, likeOp); + public ShowViewsOperation( + String catalogName, String databaseName, @Nullable ShowLikeOperator likeOp) { + this(catalogName, databaseName, null, likeOp); } - public ShowViewsOperation() { - this(null); + public ShowViewsOperation(String catalogName, String databaseName) { + this(catalogName, databaseName, null); } @Override @@ -65,17 +66,16 @@ protected String getOperationName() { protected Set retrieveDataForTableResult(Context ctx) { final CatalogManager catalogManager = ctx.getCatalogManager(); - final String catalogName = catalogManager.qualifyCatalog(this.catalogName); - final String dbName = catalogManager.qualifyDatabase(this.databaseName); if (preposition == null) { return catalogManager.listViews(); } else { Catalog catalog = catalogManager.getCatalogOrThrowException(catalogName); - if (catalog.databaseExists(dbName)) { - return catalogManager.listViews(catalogName, dbName); + if (catalog.databaseExists(databaseName)) { + return catalogManager.listViews(catalogName, databaseName); } else { throw new ValidationException( - String.format("Database '%s'.'%s' doesn't exist.", catalogName, dbName)); + String.format( + "Database '%s'.'%s' doesn't exist.", catalogName, databaseName)); } } } diff --git a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/AbstractSqlShowConverter.java b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/AbstractSqlShowConverter.java index b328f5bd4d54e..556750b082772 100644 --- a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/AbstractSqlShowConverter.java +++ b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/AbstractSqlShowConverter.java @@ -35,7 +35,12 @@ public abstract class AbstractSqlShowConverter protected Operation convertShowOperation(T sqlShowCall, ConvertContext context) { final ShowLikeOperator likeOp = getLikeOp(sqlShowCall); if (sqlShowCall.getPreposition() == null) { - return getOperationWithoutPrep(sqlShowCall, likeOp); + final CatalogManager catalogManager = context.getCatalogManager(); + final String catalogName = + catalogManager.qualifyCatalog(catalogManager.getCurrentCatalog()); + final String databaseName = + catalogManager.qualifyDatabase(catalogManager.getCurrentDatabase()); + return getOperationWithoutPrep(catalogName, databaseName, sqlShowCall, likeOp); } List sqlIdentifierNameList = sqlShowCall.getSqlIdentifierNameList(); if (sqlIdentifierNameList.size() > 2) { @@ -65,12 +70,16 @@ public ShowLikeOperator getLikeOp(SqlShowCall sqlShowCall) { sqlShowCall.getLikeSqlPattern()); } - public abstract Operation getOperationWithoutPrep(T sqlShowCall, ShowLikeOperator likeOp); + public abstract Operation getOperationWithoutPrep( + String catalogName, + String databaseName, + T sqlShowCall, + @Nullable ShowLikeOperator likeOp); public abstract Operation getOperation( T sqlShowCall, - @Nullable String catalogName, - @Nullable String databaseName, + String catalogName, + String databaseName, @Nullable String prep, @Nullable ShowLikeOperator likeOp); diff --git a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowDatabasesConverter.java b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowDatabasesConverter.java index 7b73de194cfd9..47c1ec9a781b3 100644 --- a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowDatabasesConverter.java +++ b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowDatabasesConverter.java @@ -19,6 +19,7 @@ package org.apache.flink.table.planner.operations.converters; import org.apache.flink.sql.parser.dql.SqlShowDatabases; +import org.apache.flink.table.catalog.CatalogManager; import org.apache.flink.table.operations.Operation; import org.apache.flink.table.operations.ShowDatabasesOperation; import org.apache.flink.table.operations.utils.LikeType; @@ -34,7 +35,10 @@ public Operation convertSqlNode(SqlShowDatabases sqlShowDatabases, ConvertContex LikeType.of(sqlShowDatabases.getLikeType(), sqlShowDatabases.isNotLike()), sqlShowDatabases.getLikeSqlPattern()); if (sqlShowDatabases.getPreposition() == null) { - return new ShowDatabasesOperation(likeOp); + final CatalogManager catalogManager = context.getCatalogManager(); + final String currentCatalogName = catalogManager.getCurrentCatalog(); + final String qualifiedCatalogName = catalogManager.qualifyCatalog(currentCatalogName); + return new ShowDatabasesOperation(qualifiedCatalogName, likeOp); } else { return new ShowDatabasesOperation( sqlShowDatabases.getCatalogName(), sqlShowDatabases.getPreposition(), likeOp); diff --git a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowFunctionsConverter.java b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowFunctionsConverter.java index 910ae0466539a..e6195ce76bff6 100644 --- a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowFunctionsConverter.java +++ b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowFunctionsConverter.java @@ -29,9 +29,13 @@ public class SqlShowFunctionsConverter extends AbstractSqlShowConverter { @Override - public Operation getOperationWithoutPrep(SqlShowTables sqlShowCall, ShowLikeOperator likeOp) { - return new ShowTablesOperation(likeOp); + public Operation getOperationWithoutPrep( + String qualifiedCatalogName, + String qualifiedDatabaseName, + SqlShowTables sqlShowCall, + ShowLikeOperator likeOp) { + return new ShowTablesOperation(qualifiedCatalogName, qualifiedDatabaseName, likeOp); } @Override diff --git a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowViewsConverter.java b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowViewsConverter.java index 0d8a188babee2..a214fbf4d3b45 100644 --- a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowViewsConverter.java +++ b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlShowViewsConverter.java @@ -25,8 +25,12 @@ public class SqlShowViewsConverter extends AbstractSqlShowConverter { @Override - public Operation getOperationWithoutPrep(SqlShowViews sqlShowCall, ShowLikeOperator likeOp) { - return new ShowViewsOperation(likeOp); + public Operation getOperationWithoutPrep( + String qualifiedCatalogName, + String qualifiedDatabaseName, + SqlShowViews sqlShowCall, + ShowLikeOperator likeOp) { + return new ShowViewsOperation(qualifiedCatalogName, qualifiedDatabaseName, likeOp); } @Override