[GitHub] [flink-table-store] JingsongLi commented on a diff in pull request #383: [FLINK-30033] Add primary key type validation
JingsongLi commented on code in PR #383:
URL: https://github.com/apache/flink-table-store/pull/383#discussion_r1024717157
##
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/schema/SchemaManager.java:
##
@@ -157,6 +159,33 @@ public TableSchema commitNewVersion(UpdateSchema
updateSchema) throws Exception
}
}
+private void validatePrimaryKeysType(UpdateSchema updateSchema,
List primaryKeys) {
+if (!primaryKeys.isEmpty()) {
+Map rowFields = new HashMap<>();
+for (RowType.RowField rowField :
updateSchema.rowType().getFields()) {
+rowFields.put(StringUtils.lowerCase(rowField.getName()),
rowField);
Review Comment:
I created a jira for hive https://issues.apache.org/jira/browse/FLINK-29988
--
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
[GitHub] [flink-table-store] JingsongLi commented on a diff in pull request #383: [FLINK-30033] Add primary key type validation
JingsongLi commented on code in PR #383:
URL: https://github.com/apache/flink-table-store/pull/383#discussion_r1024684798
##
docs/content/docs/development/create-table.md:
##
@@ -296,6 +296,24 @@ Use approach one if you have a large number of filtered
queries
with only `user_id`, and use approach two if you have a large
number of filtered queries with only `catalog_id`.
+The primary key data types currently supported by the table store
Review Comment:
Maybe we don't need to document this.
##
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/schema/SchemaManager.java:
##
@@ -157,6 +159,33 @@ public TableSchema commitNewVersion(UpdateSchema
updateSchema) throws Exception
}
}
+private void validatePrimaryKeysType(UpdateSchema updateSchema,
List primaryKeys) {
+if (!primaryKeys.isEmpty()) {
+Map rowFields = new HashMap<>();
+for (RowType.RowField rowField :
updateSchema.rowType().getFields()) {
+rowFields.put(StringUtils.lowerCase(rowField.getName()),
rowField);
Review Comment:
Why use `lowerCase`?
##
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/schema/SchemaManager.java:
##
@@ -157,6 +159,33 @@ public TableSchema commitNewVersion(UpdateSchema
updateSchema) throws Exception
}
}
+private void validatePrimaryKeysType(UpdateSchema updateSchema,
List primaryKeys) {
+if (!primaryKeys.isEmpty()) {
+Map rowFields = new HashMap<>();
+for (RowType.RowField rowField :
updateSchema.rowType().getFields()) {
+rowFields.put(StringUtils.lowerCase(rowField.getName()),
rowField);
+}
+for (String primaryKeyName : primaryKeys) {
+RowType.RowField rowField =
rowFields.get(StringUtils.lowerCase(primaryKeyName));
+LogicalType logicalType = rowField.getType();
+if (TableSchema.PRIMARY_KEY_UNSUPPORTED_LOGICAL_TYPES.stream()
+.anyMatch(c -> c.isInstance(logicalType))) {
+throw new UnsupportedOperationException(
+String.format(
+"Don't support to create primary key in
[%s], the type of column[%s] is [%s]",
Review Comment:
`The type %s in primary key field %s is unsupported`?
--
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
[GitHub] [flink-table-store] JingsongLi commented on a diff in pull request #383: [FLINK-30033] Add primary key type validation
JingsongLi commented on code in PR #383: URL: https://github.com/apache/flink-table-store/pull/383#discussion_r1023479832 ## docs/content/docs/development/create-table.md: ## @@ -296,6 +296,24 @@ Use approach one if you have a large number of filtered queries with only `user_id`, and use approach two if you have a large number of filtered queries with only `catalog_id`. +The primary key data types currently supported by the table store +are as follows: +``` +| TinyIntType Review Comment: Can we check it in reverse? For example, users are reminded of errors in array and map. Because it may not be possible to list all types here, such as LocalZoneTimestamp and Time. -- 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
