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/04/11 11:57:57 UTC
[GitHub] [flink] lsyldliu opened a new pull request, #19424: [FLINK-22732][table] Restrict ALTER TABLE from setting empty table op…
lsyldliu opened a new pull request, #19424:
URL: https://github.com/apache/flink/pull/19424
## What is the purpose of the change
Restrict ALTER TABLE SET syntax from setting empty table options
## Brief change log
- *Restrict ALTER TABLE SET syntax from setting empty table options*
## Verifying this change
This change added tests and can be verified as follows:
*(example:)*
- *Add unit test in SqlToOperationConverterTest*
## 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 serializers: (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 introduce a new feature? (yes )
- If yes, how is the feature documented? (not documented)
--
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] lsyldliu commented on a diff in pull request #19424: [FLINK-22732][table] Restrict ALTER TABLE from setting empty table op…
Posted by GitBox <gi...@apache.org>.
lsyldliu commented on code in PR #19424:
URL: https://github.com/apache/flink/pull/19424#discussion_r858233964
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/SqlToOperationConverter.java:
##########
@@ -547,6 +547,12 @@ private Operation convertAlterTableOptions(
ObjectIdentifier tableIdentifier,
CatalogTable oldTable,
SqlAlterTableOptions alterTableOptions) {
+ Map<String, String> setOptions =
Review Comment:
I think we can follow the `ALTER TABLE ... RESET ...` which check empty also in here.
--
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 #19424: [FLINK-22732][table] Restrict ALTER TABLE from setting empty table op…
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19424:
URL: https://github.com/apache/flink/pull/19424#issuecomment-1094966308
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5e08efde2d53347b8e1cce64bbe6ef9b05b051e4",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5e08efde2d53347b8e1cce64bbe6ef9b05b051e4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5e08efde2d53347b8e1cce64bbe6ef9b05b051e4 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] luoyuxia commented on a diff in pull request #19424: [FLINK-22732][table] Restrict ALTER TABLE from setting empty table op…
Posted by GitBox <gi...@apache.org>.
luoyuxia commented on code in PR #19424:
URL: https://github.com/apache/flink/pull/19424#discussion_r851182447
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/SqlToOperationConverter.java:
##########
@@ -547,6 +547,12 @@ private Operation convertAlterTableOptions(
ObjectIdentifier tableIdentifier,
CatalogTable oldTable,
SqlAlterTableOptions alterTableOptions) {
+ Map<String, String> setOptions =
Review Comment:
Will it be better to do the validation when parse `alter table set` in `parserImpls.ftl`
For me, it looks like a restriction in sql synax level. From my sense, `alter table cat1.db1.tb1 set ()` is like a illegal sql.
Anyway, looks good to me for either way .
--
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