jelly-1203 commented on a change in pull request #18017:
URL: https://github.com/apache/flink/pull/18017#discussion_r766554432
##########
File path:
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/MergeTableLikeUtil.java
##########
@@ -426,6 +433,14 @@ private void appendDerivedColumns(
}
}
+ Set<String> physicalFieldNames =
physicalFieldNamesToTypes.keySet();
+ Set<String> metadataFieldNames =
metadataFieldNamesToTypes.keySet();
+ final Set<String> result = new
LinkedHashSet<>(physicalFieldNames);
+ result.retainAll(metadataFieldNames);
+ if (!result.isEmpty()) {
+ throw new ValidationException(
+ "A field name conflict exists between a field
of the regular type and a field of the Metadata type.");
+ }
Review comment:
> may be we can just check duplication when put the new Column to the
columns, at the end of this function?
hi @wenlong88
Thanks for your review and comments, I think we can adjust the determination
of whether a SqlRegularColumn type field is duplicated with other types of
fields to check if a new column is duplicated when put into the column at the
end of this function.
But accessibleFieldNamesToTypes heavy logic to this place, I think it is
necessary, because, when creating computecolcolumn if metadata columns in the
former, repetitive physical column in the bottom, In the use of
accessibleFieldNamesToTypes putAll, metadata list field covers physical column,
if the user want, according to the calculated regularColumn you don't need to
do so.
##########
File path:
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/MergeTableLikeUtil.java
##########
@@ -426,6 +433,14 @@ private void appendDerivedColumns(
}
}
+ Set<String> physicalFieldNames =
physicalFieldNamesToTypes.keySet();
+ Set<String> metadataFieldNames =
metadataFieldNamesToTypes.keySet();
+ final Set<String> result = new
LinkedHashSet<>(physicalFieldNames);
+ result.retainAll(metadataFieldNames);
+ if (!result.isEmpty()) {
+ throw new ValidationException(
+ "A field name conflict exists between a field
of the regular type and a field of the Metadata type.");
+ }
Review comment:
> may be we can just check duplication when put the new Column to the
columns, at the end of this function?
hi @wenlong88
Thanks for your review and comments, I think we can adjust the determination
of whether a SqlRegularColumn type field is duplicated with other types of
fields to check if a new column is duplicated when put into the column at the
end of this function.
But accessibleFieldNamesToTypes heavy logic to this place, I think it is
necessary, because, when creating computecolcolumn if metadata columns in the
former, repetitive physical column in the bottom, In the use of
accessibleFieldNamesToTypes putAll, metadata list field covers physical column,
if the user want, according to the calculated regularColumn you don't need to
do so.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
