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/12/14 08:25:31 UTC
[GitHub] [flink] LadyForest opened a new pull request, #21507: [FLINK-30386] Fix the issue that column constraint lacks primary key not enforced check
LadyForest opened a new pull request, #21507:
URL: https://github.com/apache/flink/pull/21507
## What is the purpose of the change
This pull request aims to fix the issue that the column constraint lacks the not enforced check.
```sql
CREATE TABLE foo (
a INT PRIMARY KEY, -- currently this passes the validation, but it should be `a INT PRIMARY KEY NOT ENFORCED`
b STRING
) WITH (...)
```
## Brief change log
Currently, the constraint check(on the duplicate primary key, unique key, and enforced mode) spreads over the parse and conversion phase(see `SqlCreateTable#validate` and `SqlToOperatoinConverter#validateTableConstraint`).
To improve this, introduce a `SqlConstraintValidator` to perform the unified check for both column and table constraints.
The logic of`SqlToOperatoinConverter#validateTableConstraint` is moved to `SqlConstraintValidator`.
## Verifying this change
This change added tests and can be verified as follows:
- FlinkSqlParserImplTest
- SqlToOperationConverterTest
- HBaseConnectorITCase
- HiveServer2EndpointITCase
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduces a new feature? no
- If yes, how is the feature documented? not applicable
--
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] lincoln-lil commented on pull request #21507: [FLINK-30386] Fix the issue that column constraint lacks primary key not enforced check
Posted by "lincoln-lil (via GitHub)" <gi...@apache.org>.
lincoln-lil commented on PR #21507:
URL: https://github.com/apache/flink/pull/21507#issuecomment-1419010310
@flinkbot run azure
--
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] lincoln-lil merged pull request #21507: [FLINK-30386] Fix the issue that column constraint lacks primary key not enforced check
Posted by "lincoln-lil (via GitHub)" <gi...@apache.org>.
lincoln-lil merged PR #21507:
URL: https://github.com/apache/flink/pull/21507
--
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] lincoln-lil commented on pull request #21507: [FLINK-30386] Fix the issue that column constraint lacks primary key not enforced check
Posted by "lincoln-lil (via GitHub)" <gi...@apache.org>.
lincoln-lil commented on PR #21507:
URL: https://github.com/apache/flink/pull/21507#issuecomment-1420303452
@LadyForest could you also create a backport pr for 1.16?
--
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] flinkbot commented on pull request #21507: [FLINK-30386] Fix the issue that column constraint lacks primary key not enforced check
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #21507:
URL: https://github.com/apache/flink/pull/21507#issuecomment-1350628274
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "24fd598c81f1bb6c13c701240956fec1acce98f3",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "24fd598c81f1bb6c13c701240956fec1acce98f3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 24fd598c81f1bb6c13c701240956fec1acce98f3 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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] lincoln-lil commented on a diff in pull request #21507: [FLINK-30386] Fix the issue that column constraint lacks primary key not enforced check
Posted by "lincoln-lil (via GitHub)" <gi...@apache.org>.
lincoln-lil commented on code in PR #21507:
URL: https://github.com/apache/flink/pull/21507#discussion_r1096998249
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/SqlConstraintValidator.java:
##########
@@ -80,4 +83,20 @@ public static void validateAndChangeColumnNullability(
}
}
}
+
+ /** Check table/column constraint. */
Review Comment:
nit: remove '/column' since only table constant is being validated?
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/SqlConstraintValidator.java:
##########
@@ -80,4 +83,20 @@ public static void validateAndChangeColumnNullability(
}
}
}
+
+ /** Check table/column constraint. */
+ private static void validate(SqlTableConstraint constraint) throws SqlValidateException {
+ if (constraint.isUnique()) {
+ throw new SqlValidateException(
+ constraint.getParserPosition(), "UNIQUE constraint is not supported yet");
+ }
+ if (constraint.isEnforced()) {
+ throw new SqlValidateException(
+ constraint.getParserPosition(),
+ "Flink doesn't support ENFORCED mode for "
+ + "PRIMARY KEY constraint. ENFORCED/NOT ENFORCED controls if the constraint checks are performed on the incoming/outgoing data. "
+ + "Flink does not own the data therefore the only supported mode "
+ + "is the NOT ENFORCED mode");
Review Comment:
nit: adjust the splitting of sentences to make each row similar in length
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/SqlConstraintValidator.java:
##########
@@ -52,21 +52,24 @@ public static List<SqlTableConstraint> getFullConstraints(
return ret;
}
- /** Check duplicate constraints and change the nullability of primary key columns. */
+ /**
+ * Check constraints and change the nullability of primary key columns.
+ *
+ * @throws SqlValidateException if encountered duplicate primary key constraints, or the
+ * constraint is enforced or unique.
+ */
public static void validateAndChangeColumnNullability(
List<SqlTableConstraint> tableConstraints, SqlNodeList columnList)
throws SqlValidateException {
- List<SqlTableConstraint> constraints =
- getFullConstraints(tableConstraints, columnList).stream()
- .filter(SqlTableConstraint::isPrimaryKey)
- .collect(Collectors.toList());
-
- if (constraints.size() > 1) {
+ List<SqlTableConstraint> fullConstraints = getFullConstraints(tableConstraints, columnList);
+ if (fullConstraints.stream().filter(SqlTableConstraint::isPrimaryKey).count() > 1) {
throw new SqlValidateException(
- constraints.get(1).getParserPosition(), "Duplicate primary key definition");
- } else if (constraints.size() == 1) {
+ fullConstraints.get(1).getParserPosition(), "Duplicate primary key definition");
+ }
+ for (SqlTableConstraint constraint : fullConstraints) {
+ validate(constraint);
Set<String> primaryKeyColumns =
- Arrays.stream(constraints.get(0).getColumnNames()).collect(Collectors.toSet());
+ Arrays.stream(constraint.getColumnNames()).collect(Collectors.toSet());
Review Comment:
add comment to state the nullability rewriting here or extract these lines to a single method `rewriteNullability` ?
--
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] LadyForest commented on pull request #21507: [FLINK-30386] Fix the issue that column constraint lacks primary key not enforced check
Posted by "LadyForest (via GitHub)" <gi...@apache.org>.
LadyForest commented on PR #21507:
URL: https://github.com/apache/flink/pull/21507#issuecomment-1420061832
@flinkbot run azure
--
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] LadyForest commented on pull request #21507: [FLINK-30386] Fix the issue that column constraint lacks primary key not enforced check
Posted by GitBox <gi...@apache.org>.
LadyForest commented on PR #21507:
URL: https://github.com/apache/flink/pull/21507#issuecomment-1352387719
@flinkbot run azure
--
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