Skip to content
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

[Improvement]: Support computed columns for mixed-format tables in Flink DDL #1457

Closed
2 of 3 tasks
Tracked by #2176
YesOrNo828 opened this issue May 22, 2023 · 8 comments · Fixed by #2239
Closed
2 of 3 tasks
Tracked by #2176

[Improvement]: Support computed columns for mixed-format tables in Flink DDL #1457

YesOrNo828 opened this issue May 22, 2023 · 8 comments · Fixed by #2239
Assignees
Labels
module:mixed-flink Flink moduel for Mixed Format type:improvement

Comments

@YesOrNo828
Copy link
Contributor

Search before asking

  • I have searched in the issues and found no similar issues.

What would you like to be improved?

create table arctic.test.L (id int, proc as PROCTIME())
Caused by: java.lang.UnsupportedOperationException: Creating table with computed columns is not supported yet.
	at com.netease.arctic.flink.catalog.ArcticCatalog.lambda$validateFlinkTable$3(ArcticCatalog.java:328)
	at java.util.ArrayList.forEach(ArrayList.java:1257)
	at com.netease.arctic.flink.catalog.ArcticCatalog.validateFlinkTable(ArcticCatalog.java:326)
	at com.netease.arctic.flink.catalog.ArcticCatalog.createTable(ArcticCatalog.java:271)

How should we improve?

Filter the computed column or watermark definition field.

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Subtasks

No response

Code of Conduct

@YesOrNo828 YesOrNo828 added type:improvement module:mixed-flink Flink moduel for Mixed Format labels May 22, 2023
@YesOrNo828 YesOrNo828 changed the title [Improvement]: Support defining the computed columns in Flink DDL [Improvement]: Support computed columns in Flink DDL May 22, 2023
@wuwenchi
Copy link

Interested in it.
A PR for this feature has been mentioned to iceberg, but iceberg may be reluctant to support this feature.
(apache/iceberg#4625. Record expressions through table properties.)
Have you finalized your plans now?

@YesOrNo828
Copy link
Contributor Author

Have you finalized your plans now?

@wuwenchi Yes, the Iceberg doesn't support this feature, but this is a really useful feature. Currently, the Arctic is available to add watermark strategies or computed columns via flink SQL: CREATE TABLE LIKE. But needs another step to create a temporal table to achieve this feature.
BTW, Apache Paimon also supports computed columns and stores expressions in the table properties.

Could you show your proposal here?

@YesOrNo828
Copy link
Contributor Author

I think when a mixed-format table is created through the Flink catalog, the virtual columns and watermarks could be serialized into the options.
Deserializing to the flink table's schema from the options when get the mixed format table through the Flink Catalog.

@xieyi888
Copy link
Contributor

@YesOrNo828 hi, I'm interested in this issue, Could you assign it to me?

@YesOrNo828
Copy link
Contributor Author

@YesOrNo828 hi, I'm interested in this issue, Could you assign it to me?

Done, looking forward to your contribution.

@xieyi888
Copy link
Contributor

xieyi888 commented Nov 2, 2023

I think when a mixed-format table is created through the Flink catalog, the virtual columns and watermarks could be serialized into the options. Deserializing to the flink table's schema from the options when get the mixed format table through the Flink Catalog.

Yes, Here is the key point

  1. suppoort serialized and deserializing compute columns and watermarks
  2. Only convert flink physicalColums into arctic(iceberg) schema during create table, compute columns and watermarks should be serialized into table properties
  3. when get table, deserializing compute columns and watermarks ,and combine with flink TableSchema

I had some confusion. I see here is an dimTable in arctic ,should dimTable also refer to the points above?
https://github.com/NetEase/amoro/blob/master/flink/v1.15/flink/src/main/java/com/netease/arctic/flink/table/DynamicTableFactory.java#L167C1-L167C85

@YesOrNo828
Copy link
Contributor Author

I had some confusion. I see here is an dimTable in arctic ,should dimTable also refer to the points above?

I think this dim table would be removed in the future. This is legacy lookup join implemented in the Amoro and would be deprecated. So I think we couldn't update it.

cc @zhoujinsong WDYT?

@zhoujinsong
Copy link
Contributor

I had some confusion. I see here is an dimTable in arctic ,should dimTable also refer to the points above?

I think this dim table would be removed in the future. This is legacy lookup join implemented in the Amoro and would be deprecated. So I think we couldn't update it.

cc @zhoujinsong WDYT?

I also think so.

@zhoujinsong zhoujinsong mentioned this issue Jun 25, 2024
66 tasks
@zhoujinsong zhoujinsong changed the title [Improvement]: Support computed columns in Flink DDL [Improvement]: Support computed columns for mixed-format tables in Flink DDL Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:mixed-flink Flink moduel for Mixed Format type:improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants