Return CATALOG/SCHEMA_NOT_FOUND for SHOW CREATE statements #23197
Conversation
|
@ebyhr I wanted to see what tests will fail first :) |
wendigo
changed the title
wendigo
force-pushed
the
serafin/improve-error-message
branch
from
8f28612
to
4b7a7e0
Compare
wendigo
changed the title
wendigo
changed the title
wendigo
force-pushed
the
serafin/improve-error-message
branch
from
4b7a7e0
to
bcf873d
Compare
wendigo
force-pushed
the
serafin/improve-error-message
branch
from
bcf873d
to
82242f6
Compare
| @@ -397,6 +397,10 @@ protected Node visitShowSchemas(ShowSchemas node, Void context) | |||
| } | |||
|
|
|||
| String catalog = node.getCatalog().map(Identifier::getValue).orElseGet(() -> session.getCatalog().orElseThrow()); | |||
| if (!metadata.catalogExists(session, catalog)) { | |||
There was a problem hiding this comment.
Does this change the security behavior in the case where io.trino.spi.security.SystemAccessControl#canAccessCatalog returns false?
I see that method is called from every method in AccessControlManager, so the below checkCanShowSchemas() would fail with Cannot access catalog if the user doesn't have access. After this change, would SHOW SCHEMAS start failing with CATALOG_NOT_FOUND instead of ACCESS_DENIED?
There was a problem hiding this comment.
Right now the code is inconsistent in different SHOW CREATE cases (i.e. SHOW CREATE SCHEMA checks for schema existence. I agree that the SHOW CREATE statements shouldn't reveal whether relation exists (even if the user has access to it) so I think that in both cases - relation does not exist or user has no access to it, we should throw not found exception. WDYT?
There was a problem hiding this comment.
@dain What do you think is the right behavior?
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
|
Accidentally closed ? |
|
No, there is no consensus what should be the direction |
Fixes #23193
Description
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
(x) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: