wuchong commented on a change in pull request #15585:
URL: https://github.com/apache/flink/pull/15585#discussion_r616477380
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java
##########
@@ -325,11 +328,13 @@ private boolean executeStatement(String statement,
ExecutionMode executionMode)
try {
final Optional<Operation> operation = parseCommand(statement);
operation.ifPresent(op -> callOperation(op, executionMode));
+ return true;
+ } catch (SqlParseException e) {
Review comment:
If we want to distinguish parse error messages, we don't need to
introduce a new exception class. We can just catch exceptions for
`parseCommand`.
The class hierarchy of `SqlClientException`, `SqlExecutionException`,
`SqlParseException` looks confusing now, I mean it's not clear when to use
which exception class.
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/local/LocalExecutor.java
##########
@@ -175,10 +176,10 @@ public Operation parseStatement(String sessionId, String
statement)
try {
operations = context.wrapClassLoader(() ->
parser.parse(statement));
} catch (Exception e) {
- throw new SqlExecutionException("Failed to parse statement: " +
statement, e);
+ throw new SqlParseException("Failed to parse statement: " +
statement, e);
Review comment:
Personally, I think this pull request still not resolve the problem of
the issue. You will still get the reason
`org.apache.calcite.runtime.CalciteException: Non-query expression encountered
in illegal context` when entering `invalid command;`. However, the reason
message still can't help users figure out what happens.
I think the purpose of this issue is reminding users this is an unsupported
syntax. In order to improve the exception message, we can catch exception here,
and wrap it into a new exception with message "Unsupported statement syntax is
encountered" (or sth. similar) when the exception class is `CalciteException`
and the exception message is matched.
--
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:
us...@infra.apache.org
