liuml07 commented on code in PR #26781:
URL: https://github.com/apache/flink/pull/26781#discussion_r2203895679
##########
flink-table/flink-sql-gateway/src/main/java/org/apache/flink/table/gateway/service/operation/OperationExecutor.java:
##########
@@ -180,28 +182,28 @@ public ResultFetcher configureSession(OperationHandle
handle, String statement)
}
Operation op = parsedOperations.get(0);
- if (!(op instanceof SetOperation)
- && !(op instanceof ResetOperation)
- && !(op instanceof CreateOperation)
- && !(op instanceof DropOperation)
- && !(op instanceof UseOperation)
- && !(op instanceof AlterOperation)
- && !(op instanceof LoadModuleOperation)
- && !(op instanceof UnloadModuleOperation)
- && !(op instanceof AddJarOperation)) {
+ // Use LinkedHashMap to preserve insertion order
+ Map<Class<?>, String> ops = new LinkedHashMap<>();
+ ops.put(SetOperation.class, "SET");
+ ops.put(ResetOperation.class, "RESET");
+ ops.put(
+ CreateOperation.class,
+ "CREATE TABLE, CREATE DATABASE, CREATE FUNCTION, CREATE
CATALOG, CREATE VIEW");
+ ops.put(
+ DropOperation.class,
+ "DROP TABLE, DROP DATABASE, DROP FUNCTION, DROP CATALOG, DROP
VIEW");
+ ops.put(AlterOperation.class, "ALTER TABLE, ALTER DATABASE, ALTER
FUNCTION");
+ ops.put(UseOperation.class, "USE CATALOG, USE [CATALOG.]DATABASE, USE
MODULE");
+ ops.put(LoadModuleOperation.class, "LOAD MODULE");
+ ops.put(UnloadModuleOperation.class, "UNLOAD MODULE");
+ ops.put(AddJarOperation.class, "ADD JAR");
+
+ if (ops.keySet().stream().noneMatch(c -> c.isInstance(op))) {
throw new UnsupportedOperationException(
String.format(
- "Unsupported statement for configuring
session:%s\n"
- + "The configureSession API only supports
to execute statement of type "
- + "CREATE TABLE, DROP TABLE, ALTER TABLE, "
- + "CREATE DATABASE, DROP DATABASE, ALTER
DATABASE, "
- + "CREATE FUNCTION, DROP FUNCTION, ALTER
FUNCTION, "
- + "CREATE CATALOG, DROP CATALOG, "
- + "USE CATALOG, USE [CATALOG.]DATABASE, "
- + "CREATE VIEW, DROP VIEW, "
- + "LOAD MODULE, UNLOAD MODULE, USE MODULE,
"
- + "ADD JAR.",
- statement));
+ "Unsupported statement for configuring session:
%s\n"
Review Comment:
Great question. And thanks for reviewing!
I started with a simple fix of the incomplete error message and later a few
ad hoc Java improvements when I read the code for the patch. Those are simple
to implement so I didn't use AI tooling.
When addressing the specific comment of putting things in a variable, I
found it is not straightforward to automatically "infer" the error message from
all supported operations. I asked AI assistant and it gave me a much bigger
change by defining "getting operation name" for all operations. I think that is
too much for just error message constructing. So I went with the current
approach , which is still better than existing code as it couples the supported
operations with the error message by a single local variable.
--
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]
