gustavodemorais commented on code in PR #27854:
URL: https://github.com/apache/flink/pull/27854#discussion_r3014564683
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/materializedtable/SqlCreateOrAlterMaterializedTableConverter.java:
##########
@@ -198,23 +198,40 @@ private List<TableChange> getSchemaTableChanges(
final ResolvedCatalogMaterializedTable oldTable) {
final ResolvedSchema oldSchema = oldTable.getResolvedSchema();
final ResolvedSchema newSchema =
schemaResolver.resolve(mergeContext.getMergedSchema());
+ final boolean hasSchemaDefinition = mergeContext.hasSchemaDefinition();
final List<TableChange> changes =
new ArrayList<>(
MaterializedTableUtils.validateAndExtractColumnChanges(
- oldSchema, newSchema,
mergeContext.hasSchemaDefinition()));
+ oldSchema, newSchema, hasSchemaDefinition));
+ getConstraintChange(oldSchema, newSchema,
mergeContext.hasConstraintDefinition())
+ .ifPresent(changes::add);
+ getWatermarkChange(oldSchema, newSchema,
hasSchemaDefinition).ifPresent(changes::add);
+ return changes;
+ }
+
+ private Optional<TableChange> getConstraintChange(
+ final ResolvedSchema oldSchema,
+ final ResolvedSchema newSchema,
+ boolean hasConstraintDefinition) {
Review Comment:
```suggestion
final boolean hasConstraintDefinition) {
```
nit: both other function params above and variables below already use final.
So suggest to use final to keep the use consistent
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/materializedtable/SqlCreateOrAlterMaterializedTableConverter.java:
##########
@@ -261,6 +277,15 @@ public boolean hasSchemaDefinition() {
&& sqlNodeList.getList().get(0) instanceof
SqlRegularColumn;
}
+ @Override
+ public boolean hasConstraintDefinition() {
+ if
(!sqlCreateMaterializedTable.getTableConstraints().isEmpty()) {
Review Comment:
I think you could simplify the whole function to
```
return !sqlCreateMaterializedTable.getTableConstraints().isEmpty() ||
hasSchemaDefinition();
```
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlNodeToOperationSqlCreateOrAlterMaterializedTableConverterTest.java:
##########
@@ -283,7 +282,7 @@ void
testCreateOrAlterMaterializedTableWithDistributionForExistingTable()
@Test
void testCreateOrAlterMaterializedTableWithDroppedConstraint() {
Review Comment:
nit
```suggestion
void
testCreateOrAlterMaterializedTableWithDroppedConstraintWithExplicitSchema() {
```
--
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]
