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

Remove Iceberg, Delta $data system table #16650

Merged
merged 5 commits into from
Mar 23, 2023

Conversation

findepi
Copy link
Member

@findepi findepi commented Mar 21, 2023

It was not intentional to expose a table's data as a_table$data "system" table. This commit removes support for these tables.

findepi added 2 commits March 21, 2023 16:00
Fail, instead of returning, on an impossible case that was supposed to
be handled earlier in a method.
Leverage information already had within the method.
@findepi
Copy link
Member Author

findepi commented Mar 22, 2023

CI #13995
#6515 (will workaround)

@findepi findepi force-pushed the findepi/remove-iceberg-delta-data-system-table-98c84d branch from f733378 to 2b1f5e4 Compare March 22, 2023 08:16
@Override
public void testNoDataSystemTable()
{
// TODO (https://github.com/trinodb/trino/issues/6515): Big Query throws an error when trying to read "some_table$data".
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions github-actions bot added the bigquery BigQuery connector label Mar 22, 2023
Copy link
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skimmed, LGTM

@ebyhr
Copy link
Member

ebyhr commented Mar 22, 2023

It was not intentional to expose a table's data as a_table$data "system" table.

It has been documented in Iceberg connector since version 370 (#10514). I don't disagree with this change, but some users may use it. Could you update both delta-lake.rst and iceberg.rst?

@findepi findepi force-pushed the findepi/remove-iceberg-delta-data-system-table-98c84d branch from 2b1f5e4 to 3d984d8 Compare March 22, 2023 16:54
@findepi
Copy link
Member Author

findepi commented Mar 22, 2023

@github-actions github-actions bot added the docs label Mar 22, 2023
findepi added 3 commits March 22, 2023 22:17
It was not intentional to expose a table's data as `a_table$data`
"system" table. This commit removes support for these tables.
Encapsulate constructors of IcebergTableName and DeltaLakeTableName. The
classes are used primarily as utility classes. Constructor encapsulation
is preparation to convert them into proper utility classes.
After recent changes it became used only in tests. This also converts
the IcebergTableName, DeltaLakeTableName into utility classes.
@findepi findepi force-pushed the findepi/remove-iceberg-delta-data-system-table-98c84d branch from 3d984d8 to 3f9806d Compare March 22, 2023 21:18
@findepi
Copy link
Member Author

findepi commented Mar 23, 2023

CI #15429 cc @elonazoulay

@findepi findepi merged commit 78473a5 into master Mar 23, 2023
@findepi findepi deleted the findepi/remove-iceberg-delta-data-system-table-98c84d branch March 23, 2023 14:38
@findepi findepi mentioned this pull request Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bigquery BigQuery connector cla-signed delta-lake Delta Lake connector docs iceberg Iceberg connector
Development

Successfully merging this pull request may close these issues.

3 participants