-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[BugFix] Support catalog name in Show materialized views #56174
base: main
Are you sure you want to change the base?
Conversation
@@ -310,7 +310,14 @@ public ShowResultSet visitShowStatement(ShowStmt statement, ConnectContext conte | |||
@Override | |||
public ShowResultSet visitShowMaterializedViewStatement(ShowMaterializedViewsStmt statement, ConnectContext context) { | |||
String dbName = statement.getDb(); | |||
Database db = GlobalStateMgr.getCurrentState().getLocalMetastore().getDb(dbName); | |||
String catalogName = statement.getCatalogName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to update/fix ShowExecutor
for show materialized views
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because ShowExecutor handles the SHOW command including show materialised views.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh. Somehow I thought ShowExecutor
is for show backends
for the backend/executor nodes. This is for executing show
statements.
7695997
to
038fa62
Compare
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]❌ fail : 12 / 16 (75.00%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
Signed-off-by: Amogh Margoor <[email protected]>
038fa62
to
ea1e8d7
Compare
@murphyatwork Just added UT for more code coverage. Can you reapprove it ? |
Why I'm doing:
Show Materialized View should honor fully qualified name of database ie.., catalog_name.database_name which fails currently
mysql> mysql> show materialized views from default_catalog.test; ERROR 5501 (3F000): Getting analyzing error. Detail message: Unknown database 'default_catalog.test'. mysql> mysql>
What I'm doing:
In this patch the catalog name will be accepted as a part of fully qualified database name in
SHOW MATERIALIZED VIEW
.Fixes #54134
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: