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

Add redirection awareness for ADD COLUMN, COMMENT, DROP table tasks #11072

Conversation

findinpath
Copy link
Contributor

@findinpath findinpath commented Feb 16, 2022

Description

Add redirection awareness for the following tasks:

  • ALTER TABLE XXX ADD COLUMN
  • COMMENT
  • DROP TABLE

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

This is a fix.

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

This is a core engine change affecting mainly hive/iceberg connectors.

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

In the context of dealing with a shared Hive Metastore Service where tables of different types can co-exist in the metastore, it is helpful for the end-user to perform automatically table redirections when the user is in the context of a hive schema, but is referencing an iceberg table.

Related issues, pull requests, and links

This is a spin-off from the PR: #10577

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:

# Core
* Add redirection awareness for `ADD COLUMN`, `DROP TABLE`, `COMMENT` tasks.

Relates to #11202

@cla-bot cla-bot bot added the cla-signed label Feb 16, 2022
@findinpath findinpath force-pushed the add-redirection-awareness-add-column-comment-drop-table branch from 324e709 to 1bef4af Compare February 16, 2022 20:29
@findinpath findinpath force-pushed the add-redirection-awareness-add-column-comment-drop-table branch from 28adcd6 to 39f330a Compare February 22, 2022 09:10
QualifiedObjectName originalTableName = createQualifiedObjectName(session, statement, statement.getName());
RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, originalTableName);
if (redirectionAwareTableHandle.getTableHandle().isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table does not exist: %s", originalTableName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need to wrap the throw with a statement existence check?

if (!statement.isTableExists()) {
    throw semanticException...
}
return immediateVoidFuture();

QualifiedObjectName originalTableName = createQualifiedObjectName(session, statement, prefix.get());
RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, originalTableName);
if (redirectionAwareTableHandle.getTableHandle().isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table does not exist: " + originalTableName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reminder for statement existence check.

@findinpath findinpath force-pushed the add-redirection-awareness-add-column-comment-drop-table branch from 39f330a to f62c8ec Compare February 22, 2022 09:16
@findinpath
Copy link
Contributor Author

Rebasing on master to make use of the changes from #11143

@findinpath findinpath force-pushed the add-redirection-awareness-add-column-comment-drop-table branch from f62c8ec to 5347c09 Compare February 23, 2022 14:45
@findinpath findinpath requested a review from phd3 February 23, 2022 14:46
@findepi findepi requested a review from losipiuk February 24, 2022 14:30
@findepi
Copy link
Member

findepi commented Feb 25, 2022

@phd3 do you want to take another look?

@findinpath findinpath force-pushed the add-redirection-awareness-add-column-comment-drop-table branch from 5347c09 to 86ad52a Compare February 28, 2022 13:52
@findinpath findinpath changed the title Add redirection awareness for table tasks Add redirection awareness for ADD COLUMN, COMMENT, DROP table tasks Feb 28, 2022
@findepi findepi requested a review from phd3 February 28, 2022 13:58
Copy link
Member

@phd3 phd3 left a comment

Choose a reason for hiding this comment

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

LGTM % pending CI

@phd3
Copy link
Member

phd3 commented Mar 1, 2022

unrelated #11245

@phd3 phd3 merged commit bb58f10 into trinodb:master Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants