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

Clarify ConnectorMetadata listing when schema name not found #14592

Conversation

findepi
Copy link
Member

@findepi findepi commented Oct 12, 2022

It's important to require that listTables, listViews and listMaterializedViews do not fail when asked to list relations from a schema that does not exist. This is because schema existence cannot be ensured in a concurrent environment, i.e. a schema could have just been deleted concurrently. Lack of such assumptions for these methods would make predicate pushdown for tables like system.jdbc.tables impossible to do properly.

It's important to require that `listTables`, `listViews` and
`listMaterializedViews` do not fail when asked to list relations from a
schema that does not exist. This is because schema existence cannot be
ensured in a concurrent environment, i.e. a schema could have just been
deleted concurrently. Lack of such assumptions for these methods would
make predicate pushdown for tables like `system.jdbc.tables` impossible
to do properly.
@findepi findepi added the no-release-notes This pull request does not require release notes entry label Oct 12, 2022
@cla-bot cla-bot bot added the cla-signed label Oct 12, 2022
Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

There are already lot of non-compliant ConnectorMetadata impls today in this case.

Is this part of some larger effort or just explicitly documenting expectations of the engine?

I can create some issues for the non-compliant ones accordingly.

LGTM % my question

@findepi
Copy link
Member Author

findepi commented Oct 13, 2022

There are already lot of non-compliant ConnectorMetadata impls today in this case.

indeed, i see 6 build failures

Is this part of some larger effort or just explicitly documenting expectations of the engine?

no larger effort.
due to a support question, i almost made a change in the other direction, so this javadoc change is a tribute to myself, supposed preventing time waste next time

@findepi findepi self-assigned this Oct 14, 2022
@findepi findepi merged commit fe5482b into trinodb:master Oct 15, 2022
@findepi findepi deleted the findepi/clarify-connectormetadata-listing-when-schema-name-not-found-f08319 branch October 15, 2022 21:27
@github-actions github-actions bot added this to the 401 milestone Oct 15, 2022
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.

3 participants