Skip to content

add "STORAGE TABLE" and "MATERIALIZED VIEW" type in information_schema.tables #10745

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.

Sign up for GitHub

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

Closed
duoluodexiaokeke wants to merge 2 commits into from
+32 −2
Closed

add "STORAGE TABLE" and "MATERIALIZED VIEW" type in information_schema.tables #10745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f2cd34
add "STORAGE TABLE" and "MATERIALIZED VIEW" type in
duoluodexiaokeke Jan 22, 2022
e851d36
Merge branch 'trinodb:master' into trino-369-add_table_type
duoluodexiaokeke Jan 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion ...-main/src/main/java/io/trino/connector/informationschema/InformationSchemaPageSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@
import static io.trino.connector.informationschema.InformationSchemaMetadata.defaultPrefixes;
import static io.trino.connector.informationschema.InformationSchemaMetadata.isTablesEnumeratingTable;
import static io.trino.metadata.MetadataListing.getViews;
import static io.trino.metadata.MetadataListing.listMaterializedViews;
import static io.trino.metadata.MetadataListing.listSchemas;
import static io.trino.metadata.MetadataListing.listStorageTab;
import static io.trino.metadata.MetadataListing.listTableColumns;
import static io.trino.metadata.MetadataListing.listTablePrivileges;
import static io.trino.metadata.MetadataListing.listTables;
Expand Down Expand Up @@ -280,11 +282,23 @@ private void addTablesRecords(QualifiedTablePrefix prefix)
{
Set<SchemaTableName> tables = listTables(session, metadata, accessControl, prefix);
Set<SchemaTableName> views = listViews(session, metadata, accessControl, prefix);
Set<SchemaTableName> mviews = listMaterializedViews(session, metadata, accessControl, prefix);
Set<SchemaTableName> storageTables = listStorageTab(session, metadata, accessControl, prefix);
// TODO (https://github.com/trinodb/trino/issues/8207) define a type for materialized views

for (SchemaTableName name : union(tables, views)) {
// if table and view names overlap, the view wins
String type = views.contains(name) ? "VIEW" : "BASE TABLE";
String type = "BASE TABLE";
if (views.contains(name)) {
type = "VIEW";
}
else if (mviews.contains(name)) {
type = "MATERIALIZE VIEW";
Copy link
Copy Markdown
Member

@raunaqmorarka raunaqmorarka Jan 24, 2022

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

@findepi findepi Jan 24, 2022

Choose a reason for hiding this comment

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

If we want to follow spec, we should return "VIEW" for materialized views.
but let's wait maybe @martint has more thoughts on this.

Copy link
Copy Markdown
Contributor Author

@duoluodexiaokeke duoluodexiaokeke Jan 25, 2022

Choose a reason for hiding this comment

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

materialize view and normal view is different, so i think they are different type

}
else if (storageTables.contains(name)) {
type = "STORAGE TABLE";
Copy link
Copy Markdown
Member

@raunaqmorarka raunaqmorarka Jan 24, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@duoluodexiaokeke duoluodexiaokeke Jan 24, 2022

Choose a reason for hiding this comment

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

base table and mv table is different.
show tables command will print storage table, but i don't think this table should show

Copy link
Copy Markdown
Member

@findepi findepi Jan 24, 2022

Choose a reason for hiding this comment

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

from tables perspective, mv's storage table is a table just as any other table.

Copy link
Copy Markdown
Contributor Author

@duoluodexiaokeke duoluodexiaokeke Jan 25, 2022

Choose a reason for hiding this comment

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

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;

Copy link
Copy Markdown
Member

@findepi findepi Jan 27, 2022

Choose a reason for hiding this comment

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

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.

}

addRecord(
prefix.getCatalogName(),
name.getSchemaName(),
Expand Down
12 changes: 12 additions & 0 deletions core/trino-main/src/main/java/io/trino/metadata/MetadataListing.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.trino.connector.CatalogName;
import io.trino.security.AccessControl;
import io.trino.spi.TrinoException;
import io.trino.spi.connector.CatalogSchemaTableName;
import io.trino.spi.connector.ColumnMetadata;
import io.trino.spi.connector.SchemaTableName;
import io.trino.spi.connector.TableColumnsMetadata;
Expand Down Expand Up @@ -160,6 +161,17 @@ public static Map<SchemaTableName, ViewInfo> getMaterializedViews(Session sessio
.collect(toImmutableMap(Entry::getKey, Entry::getValue));
}

public static Set<SchemaTableName> listStorageTab(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix)
{
Set<SchemaTableName> storageTables = metadata.getMaterializedViews(session, prefix).values().stream()
.map(ViewInfo::getStorageTable)
.map(Optional<CatalogSchemaTableName>::get)
.map(CatalogSchemaTableName::getSchemaTableName)
.collect(toImmutableSet());

return accessControl.filterTables(session.toSecurityContext(), prefix.getCatalogName(), storageTables);
}

public static Set<GrantInfo> listTablePrivileges(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix)
{
List<GrantInfo> grants = metadata.listTablePrivileges(session, prefix);
Expand Down
6 changes: 5 additions & 1 deletion core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import io.trino.sql.tree.AstVisitor;
import io.trino.sql.tree.BooleanLiteral;
import io.trino.sql.tree.ColumnDefinition;
import io.trino.sql.tree.ComparisonExpression;
import io.trino.sql.tree.CreateMaterializedView;
import io.trino.sql.tree.CreateSchema;
import io.trino.sql.tree.CreateTable;
Expand Down Expand Up @@ -143,6 +144,7 @@
import static io.trino.sql.analyzer.TypeSignatureTranslator.toSqlType;
import static io.trino.sql.tree.BooleanLiteral.FALSE_LITERAL;
import static io.trino.sql.tree.BooleanLiteral.TRUE_LITERAL;
import static io.trino.sql.tree.ComparisonExpression.Operator.NOT_EQUAL;
import static io.trino.sql.tree.CreateView.Security.DEFINER;
import static io.trino.sql.tree.CreateView.Security.INVOKER;
import static io.trino.sql.tree.LogicalExpression.and;
Expand Down Expand Up @@ -275,7 +277,9 @@ protected Node visitShowTables(ShowTables showTables, Void context)
}

Expression predicate = equal(identifier("table_schema"), new StringLiteral(schema.getSchemaName()));

Expression predicateNot = new ComparisonExpression(NOT_EQUAL,
identifier("table_type"), new StringLiteral("STORAGE TABLE"));
predicate = logicalAnd(predicate, predicateNot);
Optional<String> likePattern = showTables.getLikePattern();
if (likePattern.isPresent()) {
Expression likePredicate = new LikePredicate(
Expand Down