[FLINK-38087][sql-gateway] Improve OperationExecutor error messages and Java stream usage

[liuml07]

https://issues.apache.org/jira/browse/FLINK-38087

What is the purpose of the change

The configureSession() error message does not include SET/RESET. There are also some usages of Java stream which can be improved.

This diff fixes them.

Brief change log

(for example:)

• Add SET/RESET to the error message of configureSession()
• Use EnumSet instead of Arrays->List for set contains operations
• Use Collectors.toUnmodifiableSet to collect from Java streams instead of wrapping a HashSet
• Use Optional.isEmpty instead of !Optional.isPresent()

Verifying this change

Using existing unit and integration tests.

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): (yes / no)
• The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
• The serializers: (yes / no / don't know)
• The runtime per-record code paths (performance sensitive): (yes / no / don't know)
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / no / don't know)
• The S3 file system connector: (yes / no / don't know)

Documentation

• Does this pull request introduce a new feature? (yes / no)
• If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

[liuml07]

@fsk119 Could you take a look? Thanks

[flinkbot]

CI report:

• 0a9f293 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[liuml07]

@flinkbot run azure

[davidradl]

nit: the previous line's english is not great. I suggest:
The configureSession API only supports executing statements of type

[davidradl]

nit: could we put all of these values into a variable we just iterate. Ideally the variable would be derived from what the API uses to validate.

[liuml07]

Currently there is no "operation name" available for operations. There is a asSummaryString in the interface but it's at instance level which returns the whole clause of the operation with values/properties.

Existing code has separated operation check and error messages. We can try to consolidate them with a variable - a Map of allowed Operations and their names. It would be easier when adding new operations. Let me update the code.

The main difference is, the new way does not group operations by semantics as the existing code does. For e.g. all database related messages are currently placed together, as seen "CREATE DATABASE, DROP DATABASE, ALTER DATABASE, ". I think that probably is fine if we group by operations.

[weiqingy]

Good catch! Was this PR generated with the AI assistant?

[liuml07]

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.