-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-38674][table] Support schema definition for Materialized tables while CREATE
#27222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
docs/content.zh/docs/dev/table/materialized-table/statements.md
Outdated
Show resolved
Hide resolved
|
|
||
| public boolean isSchemaWithColumnsIdentifiersOnly() { | ||
| // CREATE MATERIALIZED TABLE supports passing only column identifiers in the column list. If | ||
| // the first column in the list is an identifier, then we assume the rest of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this as called from validate, would it make sense to validate that all the columns are identifiers rather than assume?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no need for that since it is not a validation.
The validation, that if first is identifier then others are as well, happens on parse level
I submitted a test for that
https://github.com/snuyanzin/flink/blob/9963205624c657c6943fc5e726d4b1c151dfd941/flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/MaterializedTableStatementParserTest.java#L51-L57
|
|
||
| if (tableConstraint != null) { | ||
| writer.newlineAndIndent(); | ||
| if (!columnList.isEmpty() || !tableConstraints.isEmpty() || watermark != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious - can we have a watermark when there are no columns? I am not sure what a no column materialized table means - is this allowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still can have something like
CREATE MATERIALIZED TABLE users_shops (watermark for ts as ts - interval '2' SECOND)
FRESHNESS = INTERVAL '30' SECOND
AS SELECT current_timestamp() as ts, 1 as shop_id, 2 as user_id
...rg/apache/flink/table/planner/operations/converters/SqlCreateMaterializedTableConverter.java
Outdated
Show resolved
Hide resolved
...ache/flink/table/planner/operations/converters/AbstractCreateMaterializedTableConverter.java
Outdated
Show resolved
Hide resolved
cbdf074 to
9963205
Compare
...ache/flink/table/planner/operations/converters/AbstractCreateMaterializedTableConverter.java
Outdated
Show resolved
Hide resolved
dfdae67 to
97b8660
Compare
|
|
||
| ```sql | ||
| CREATE MATERIALIZED TABLE my_materialized_table_full ( | ||
| ds, product_id, product_name, avg_sale_price, total_quantity) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include a data type here? I think specifying both the name and data type is a very common task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added example below
...ink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlCreateMaterializedTable.java
Show resolved
Hide resolved
…s while `CREATE` operation This closes apache#27222.
What is the purpose of the change
This PR is starting PR for FLIP-550. Adds support for schema definition while
CREATE MATERIALIZED TABLE.Also revisits a bit hierarchy of classes for materialized table converters in a way similar to tables.
After that it will allow to have syntax in way
Brief change log
parser and materialized table converters
Verifying this change
A number of tests added for parser, converters
Does this pull request potentially affect one of the following parts:
@Public(Evolving): ( no)Documentation