Airblader commented on a change in pull request #17155:
URL: https://github.com/apache/flink/pull/17155#discussion_r704055469
##########
File path:
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/ddl/AlterTableOptionsOperation.java
##########
@@ -22,15 +22,32 @@
import org.apache.flink.table.catalog.ObjectIdentifier;
import org.apache.flink.table.operations.OperationUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Map;
import java.util.stream.Collectors;
-/** Operation to describe a ALTER TABLE .. SET .. statement. */
+/** Operation to describe a ALTER TABLE IF EXISTS .. SET .. statement. */
public class AlterTableOptionsOperation extends AlterTableOperation {
private final CatalogTable catalogTable;
+ private final Map<String, String> tableOptions;
Review comment:
Yes, other operations do that, too, and probably shouldn't do it either.
The design of operations needs to be improved overall, because both the
operations and the converter contain way too much logic. I think it would be
good to improve this for this operation with this PR, because we kind of need
it. Otherwise we'd have either incorrectness or redundancy, and both would be a
worse situation than before.
I don't think it's a big change to fix this here. It's a pretty small change
to just look up the table in `TableEnvironmentImpl` instead of passing it to
the operation during the conversion.
--
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]
