Skip to content

Retrieve the table handle only when dealing with a system table#15568

Merged
ebyhr merged 2 commits intotrinodb:masterfrom
findinpath:findinpath/iceberg-system-table-name-handling
Mar 8, 2023
Merged

Retrieve the table handle only when dealing with a system table#15568
ebyhr merged 2 commits intotrinodb:masterfrom
findinpath:findinpath/iceberg-system-table-name-handling

Conversation

@findinpath
Copy link
Copy Markdown
Contributor

@findinpath findinpath commented Dec 30, 2022

Description

This is a small usability improvement which aims to provide a more meaningful error message to the user in case that the user asks for an invalid metadata table corresponding to an existing Iceberg table.

assertQueryFailure(() -> onTrino().executeQuery("TABLE iceberg.default.\"" + tableName + "$invalid\""))
                .hasMessageMatching("\\QQuery failed (#\\E\\S+\\Q): Table '" + icebergTableName + "$invalid' redirected to '" + hiveTableName + "$invalid', but the target table '" + hiveTableName + "$invalid' does not exist");

Additional context and related issues

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

@cla-bot cla-bot Bot added the cla-signed label Dec 30, 2022
@findinpath findinpath self-assigned this Dec 30, 2022
@findinpath findinpath added no-release-notes This pull request does not require release notes entry and removed tests:hive labels Dec 30, 2022
@findinpath findinpath requested review from ebyhr and findepi and removed request for ebyhr January 4, 2023 10:36
@findinpath findinpath changed the title Check system table type only when dealing with an Iceberg table DO NOT MERGE Check system table type only when dealing with an Iceberg table Jan 18, 2023
@findepi
Copy link
Copy Markdown
Member

findepi commented Jan 18, 2023

Why is this "DO NOT MERGE"?
Does it also mean "DO NOT REVIEW"?

@findinpath
Copy link
Copy Markdown
Contributor Author

Yes, at the moment is not reviewable. I made a false supposition while writing it.

@findinpath findinpath changed the title DO NOT MERGE Check system table type only when dealing with an Iceberg table DO NOT MERGE/REVIEW Check system table type only when dealing with an Iceberg table Jan 18, 2023
@findinpath findinpath force-pushed the findinpath/iceberg-system-table-name-handling branch from 571ea1d to 218b329 Compare January 19, 2023 08:53
@findinpath findinpath changed the title DO NOT MERGE/REVIEW Check system table type only when dealing with an Iceberg table Get system table type only when dealing with an Iceberg table Jan 19, 2023
@findinpath findinpath force-pushed the findinpath/iceberg-system-table-name-handling branch 3 times, most recently from 2325b82 to f4000d4 Compare January 25, 2023 12:01
@findinpath
Copy link
Copy Markdown
Contributor Author

CI Hit #5427

@findinpath findinpath force-pushed the findinpath/iceberg-system-table-name-handling branch from a55b4d0 to 746d0e8 Compare January 27, 2023 11:42
@findinpath findinpath changed the title Get system table type only when dealing with an Iceberg table Retrieve the table handle only when dealing with a system table Jan 27, 2023
@findinpath findinpath force-pushed the findinpath/iceberg-system-table-name-handling branch from 746d0e8 to 6ea2487 Compare February 28, 2023 06:18
Comment thread plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java Outdated
@findinpath
Copy link
Copy Markdown
Contributor Author

@ebyhr CPTAL at this PR ? It is pretty similar to what has been done for the $history metadata table in Delta.

Comment thread plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableName.java Outdated
Comment thread plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableName.java Outdated
@findinpath findinpath force-pushed the findinpath/iceberg-system-table-name-handling branch from 6ea2487 to 20e802a Compare March 8, 2023 09:19
@github-actions github-actions Bot added the iceberg Iceberg connector label Mar 8, 2023
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Mar 8, 2023

CI hit #15187

@ebyhr ebyhr merged commit cd9d753 into trinodb:master Mar 8, 2023
@github-actions github-actions Bot added this to the 410 milestone Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed iceberg Iceberg connector no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

3 participants