You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/09/22 03:29:17 UTC
[GitHub] [flink-table-store] zjureel opened a new pull request, #302: [FLINK-29385] Verify duplicate column names for AddColumn
zjureel opened a new pull request, #302:
URL: https://github.com/apache/flink-table-store/pull/302
When we execute AddColumn ddl, we should check whether the column name is exist. Otherwise, the ddl will be successful and the job reads this table will create source of flink store table failed.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] SteNicholas commented on a diff in pull request #302: [FLINK-29385] Verify duplicate column names for AddColumn
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on code in PR #302:
URL: https://github.com/apache/flink-table-store/pull/302#discussion_r977219669
##########
flink-table-store-core/src/test/java/org/apache/flink/table/store/table/SchemaEvolutionTest.java:
##########
@@ -120,6 +122,28 @@ public void testAddField() throws Exception {
assertThat(rows).containsExactlyInAnyOrder(Row.of(3, 3L, 3L), Row.of(4, 4L, 4L));
}
+ @Test
+ public void testAddDuplicateField() throws Exception {
+ UpdateSchema updateSchema =
+ new UpdateSchema(
+ RowType.of(new IntType(), new BigIntType()),
+ Collections.emptyList(),
+ Collections.emptyList(),
+ new HashMap<>(),
+ "");
+ schemaManager.commitNewVersion(updateSchema);
+ schemaManager.commitChanges(
+ Collections.singletonList(SchemaChange.addColumn("f3", new BigIntType())));
+ assertThatThrownBy(
+ () -> {
+ schemaManager.commitChanges(
+ Collections.singletonList(
+ SchemaChange.addColumn("f3", new FloatType())));
+ })
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith("The column[f3] is exist in");
Review Comment:
Why not use `hasMessage` to verify the complete exception message？
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] JingsongLi merged pull request #302: [FLINK-29385] Verify duplicate column names for AddColumn
Posted by GitBox <gi...@apache.org>.
JingsongLi merged PR #302:
URL: https://github.com/apache/flink-table-store/pull/302
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] SteNicholas commented on a diff in pull request #302: [FLINK-29385] Verify duplicate column names for AddColumn
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on code in PR #302:
URL: https://github.com/apache/flink-table-store/pull/302#discussion_r977218529
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/schema/SchemaManager.java:
##########
@@ -173,6 +174,15 @@ public TableSchema commitChanges(List<SchemaChange> changes) throws Exception {
newOptions.remove(removeOption.key());
} else if (change instanceof AddColumn) {
AddColumn addColumn = (AddColumn) change;
+ Set<String> fieldNames =
+ newFields.stream().map(DataField::name).collect(Collectors.toSet());
+ if (fieldNames.contains(addColumn.fieldName())) {
+ throw new IllegalArgumentException(
+ "The column["
Review Comment:
```suggestion
String.format("The column [%s] exists in the table[%s].".format(addColumn.fieldName(), tableRoot))
```
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] zjureel commented on pull request #302: [FLINK-29385] Verify duplicate column names for AddColumn
Posted by GitBox <gi...@apache.org>.
zjureel commented on PR #302:
URL: https://github.com/apache/flink-table-store/pull/302#issuecomment-1254600956
Thanks @JingsongLi @SteNicholas I have updated the codes
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] zjureel commented on a diff in pull request #302: [FLINK-29385] Verify duplicate column names for AddColumn
Posted by GitBox <gi...@apache.org>.
zjureel commented on code in PR #302:
URL: https://github.com/apache/flink-table-store/pull/302#discussion_r977248865
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/schema/SchemaManager.java:
##########
@@ -173,6 +174,15 @@ public TableSchema commitChanges(List<SchemaChange> changes) throws Exception {
newOptions.remove(removeOption.key());
} else if (change instanceof AddColumn) {
AddColumn addColumn = (AddColumn) change;
+ Set<String> fieldNames =
Review Comment:
> Why not directly filter the field name of add column in the stream of `newFields`?
It's an interesting question. Maybe that the data types of these two columns are different, so i think it's better to return the error message to the user
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] SteNicholas commented on a diff in pull request #302: [FLINK-29385] Verify duplicate column names for AddColumn
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on code in PR #302:
URL: https://github.com/apache/flink-table-store/pull/302#discussion_r977217137
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/schema/SchemaManager.java:
##########
@@ -173,6 +174,15 @@ public TableSchema commitChanges(List<SchemaChange> changes) throws Exception {
newOptions.remove(removeOption.key());
} else if (change instanceof AddColumn) {
AddColumn addColumn = (AddColumn) change;
+ Set<String> fieldNames =
Review Comment:
Why not directly filter the field name of add column in the stream of `newFields`?
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] JingsongLi commented on a diff in pull request #302: [FLINK-29385] Verify duplicate column names for AddColumn
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on code in PR #302:
URL: https://github.com/apache/flink-table-store/pull/302#discussion_r977230937
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/schema/SchemaManager.java:
##########
@@ -173,6 +174,15 @@ public TableSchema commitChanges(List<SchemaChange> changes) throws Exception {
newOptions.remove(removeOption.key());
} else if (change instanceof AddColumn) {
AddColumn addColumn = (AddColumn) change;
+ Set<String> fieldNames =
Review Comment:
`Stream.anyMatch`
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org