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

Improve exception handling for CREATE TABLE task #11539

Merged

Conversation

findinpath
Copy link
Contributor

@findinpath findinpath commented Mar 17, 2022

Provide meaningful exception message for CREATE TABLE task
when dealing with namesake table of unsupported type in
shared metastore.

This change affects the connectors: hive, iceberg, delta lake
which may deal with a shared metastore (HMS/AWS Glue).

Description

Is this change a fix, improvement, new feature, refactoring, or other?

Fix

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

This change affects the core query engine (CreateTableTask)

How would you describe this change to a non-technical end user or system administrator?

In the context of using shared metastore between Hive, Iceberg/Delta it may very well happen that the users are trying to create a namesake table from two different connectors. This PR improves exception handling in the CREATE TABLE task to provide a meaningful message to the user in such a situation.

Related issues, pull requests, and links

Fixes #10446

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# Main
* Improve exception handling in`CREATE TABLE` task when dealing with namesake table of unsupported type in
shared metastore.

@cla-bot cla-bot bot added the cla-signed label Mar 17, 2022
@findinpath findinpath requested a review from findepi March 17, 2022 11:56
@findinpath findinpath requested review from homar and alexjo2144 March 17, 2022 11:56
@findinpath findinpath force-pushed the create-table-task-exception-handling branch from 2efb5f1 to 798422e Compare March 23, 2022 04:42
Provide meaningful exception message for `CREATE TABLE` task
when dealing with namesake table of unsupported type in
shared metastore.

This change affects the connectors: hive, iceberg, delta lake
which may deal with a shared metastore (HMS/AWS Glue).
@findinpath findinpath force-pushed the create-table-task-exception-handling branch from 798422e to e50cbc3 Compare March 23, 2022 04:42
@@ -129,6 +129,7 @@
MISSING_ROW_PATTERN(106, USER_ERROR),
INVALID_WINDOW_MEASURE(107, USER_ERROR),
STACK_OVERFLOW(108, USER_ERROR),
UNSUPPORTED_TABLE_TYPE(109, USER_ERROR),
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@alexjo2144 alexjo2144 left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

There's a bigger question along with #11617 I guess. Should connectors ever hide unsupported tables? Now that we have table redirections it seems to me like we should never filter tables, and so these create table statements should always see the existing tables rather than relying on catching it with the exception code.

@findinpath
Copy link
Contributor Author

@alexjo2144 good point.

I would rather put this PR on hold until #11617 is cleared and then act accordingly.

Calling io.trino.metadata.Metadata#listTables instead of io.trino.metadata.Metadata#getTableHandle(io.trino.Session, io.trino.metadata.QualifiedObjectName) would spare us of the exception code matching strategy which is relatively error prone.

@findepi
Copy link
Member

findepi commented Mar 25, 2022

Now that we have table redirections it seems to me like we should never filter tables, and so these create table statements should always see the existing tables rather than relying on catching it with the exception code.

getTableHandle remains to throw for an unsupported table.
A known code for "table exists but is kind of unsupported" is a new quality introduced in this PR and necessary to handle CREATE TABLE failure when redirections are not enabled (configured.)

@findepi findepi merged commit f817f0a into trinodb:master Mar 26, 2022
@findepi findepi added the no-release-notes This pull request does not require release notes entry label Mar 26, 2022
@github-actions github-actions bot added this to the 375 milestone Mar 26, 2022
@mosabua
Copy link
Member

mosabua commented Mar 28, 2022

I will assume that the label "no-release-notes" from @findepi supersedes the suggestion from @findinpath

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed no-release-notes This pull request does not require release notes entry
Development

Successfully merging this pull request may close these issues.

Misleading exception when trying to create Iceberg table if Hive table with same name already exists
4 participants