add "STORAGE TABLE" and "MATERIALIZED VIEW" type in information_schema.tables#10745
add "STORAGE TABLE" and "MATERIALIZED VIEW" type in information_schema.tables#10745duoluodexiaokeke wants to merge 2 commits intotrinodb:masterfrom
Conversation
information_schema.tables "show tables" command filter out storage table
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: liul.
|
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla. |
| type = "VIEW"; | ||
| } | ||
| else if (mviews.contains(name)) { | ||
| type = "MATERIALIZE VIEW"; |
There was a problem hiding this comment.
This was left as a TODO (see a few lines above https://github.com/trinodb/trino/pull/10745/files#diff-de0b2e0ffdaf5c2eeda3ad7f0513ad28373a158a1f4fbe32ef0a10f5ce10cc73L283) because information schema table is supposed to follow ANSI SQL spec and we didn't have clarity about which table type should be used for MVs if it is to be differentiated from the others.
fyi @sopel39 @findepi @martint
There was a problem hiding this comment.
If we want to follow spec, we should return "VIEW" for materialized views.
but let's wait maybe @martint has more thoughts on this.
There was a problem hiding this comment.
materialize view and normal view is different, so i think they are different type
| type = "MATERIALIZE VIEW"; | ||
| } | ||
| else if (storageTables.contains(name)) { | ||
| type = "STORAGE TABLE"; |
There was a problem hiding this comment.
I don't think we can give a separate type to storage table of MVs unless the ANSI SQL spec for information schema allows it.
There was a problem hiding this comment.
base table and mv table is different.
show tables command will print storage table, but i don't think this table should show
There was a problem hiding this comment.
from tables perspective, mv's storage table is a table just as any other table.
There was a problem hiding this comment.
you are right.
But from the user's point of view， they didn't create the storage tables. so these are transparent to users. they not need to find those table when used "show tabales" command;
There was a problem hiding this comment.
But from the user's point of view， they didn't create the storage tables.
It doesn't matter who created a table and why.
they not need to find those table when used "show tabales" command;
i'd suggesting filing up an issue for this to facilitate a discussion.
i will happily explain why i think we should not bar users from accessing the tables storing mv information directly.
|
👋 @duoluodexiaokeke given the discussion in #12559 and the fact that #12591 is merged, maybe this PR is no longer needed. Please chime in to the discussion there. |
|
Superseded by #15350 |
4 participants
add "STORAGE TABLE" and "MATERIALIZED VIEW" type in information_schema.tables
"show tables" command filter out storage table