LadyForest commented on code in PR #21571:
URL: https://github.com/apache/flink/pull/21571#discussion_r1059237170
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/AlterSchemaConverter.java:
##########
@@ -604,24 +703,90 @@ private void validateColumnName(
Set<String> referencedColumn =
ColumnReferenceFinder.findReferencedColumn(
computedColumn.getExpression(),
tableColumns);
- if (referencedColumn.contains(originColumnName)) {
+ if (computedColumnChecker.apply(referencedColumn,
computedColumn)) {
throw new ValidationException(
String.format(
- "Old column %s is referred by
computed column %s, currently doesn't "
- + "allow to rename
column which is referred by computed column.",
- originColumnName,
+ "%sThe column `%s` is
referenced by computed column %s.",
+ EX_MSG_PREFIX,
+ columnToAlter,
computedColumn.asSummaryString()));
}
Review Comment:
> BTW, the drop order will influence the logic here, e.g. drop physical
column first and then drop the computed column using the physical column?
I think `ALTER TABLE T DROP (col1, col2, ...)` is a bulk operation, so the
order should not influence the result. As long as the computed column is
dropped together with the physical column, it should be a valid operation. WDYT?
--
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]
