Skip to content

Fix listTables to include views #11063

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

Merged
findepi merged 2 commits into from
Feb 24, 2022
Merged

Fix listTables to include views #11063

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
6c24976
Assert consistently in testMaterializedView
findepi Feb 16, 2022
2c8bdf7
Fix listTables to include views
findepi Feb 16, 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
6 changes: 4 additions & 2 deletions plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,13 +309,15 @@ public List<SchemaTableName> listTables(ConnectorSession session, Optional<Strin
Set<String> schemaNames = filterSchema.<Set<String>>map(ImmutableSet::of)
.orElseGet(client::getSchemaNames);

ImmutableList.Builder<SchemaTableName> builder = ImmutableList.builder();
ImmutableSet.Builder<SchemaTableName> builder = ImmutableSet.builder();
for (String schemaName : schemaNames) {
for (String tableName : client.getTableNames(schemaName)) {
builder.add(new SchemaTableName(schemaName, tableName));
}
}
return builder.build();
builder.addAll(listViews(session, filterSchema));
// Deduplicate with set because state may change concurrently
return builder.build().asList();
}

@Override
Expand Down
12 changes: 8 additions & 4 deletions plugin/trino-blackhole/src/main/java/io/trino/plugin/blackhole/BlackHoleMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,14 @@ public ConnectorTableMetadata getTableMetadata(ConnectorSession session, Connect
@Override
public List<SchemaTableName> listTables(ConnectorSession session, Optional<String> schemaName)
{
return tables.values().stream()
.filter(table -> schemaName.isEmpty() || table.getSchemaName().equals(schemaName.get()))
.map(BlackHoleTableHandle::toSchemaTableName)
.collect(toList());
// Deduplicate with set because state may change concurrently
return ImmutableSet.<SchemaTableName>builder()
.addAll(tables.values().stream()
.filter(table -> schemaName.isEmpty() || table.getSchemaName().equals(schemaName.get()))
.map(BlackHoleTableHandle::toSchemaTableName)
.collect(toList()))
.addAll(listViews(session, schemaName))
.build().asList();
}

@Override
Expand Down
19 changes: 13 additions & 6 deletions plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/TrinoHiveCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import io.airlift.log.Logger;
import io.trino.plugin.base.CatalogName;
import io.trino.plugin.hive.HdfsEnvironment;
Expand Down Expand Up @@ -277,7 +278,7 @@ public Transaction newCreateTableTransaction(
@Override
public List<SchemaTableName> listTables(ConnectorSession session, Optional<String> namespace)
{
ImmutableList.Builder<SchemaTableName> tablesListBuilder = ImmutableList.builder();
ImmutableSet.Builder<SchemaTableName> tablesListBuilder = ImmutableSet.builder();
listNamespaces(session, namespace)
.stream()
.flatMap(schema -> Stream.concat(
Expand All @@ -291,8 +292,10 @@ public List<SchemaTableName> listTables(ConnectorSession session, Optional<Strin
.distinct()) // distinct() to avoid duplicates for case-insensitive HMS backends
.forEach(tablesListBuilder::add);

tablesListBuilder.addAll(listViews(session, namespace));
tablesListBuilder.addAll(listMaterializedViews(session, namespace));
return tablesListBuilder.build();
// Deduplicate with set because state may change concurrently
return tablesListBuilder.build().asList();
}

@Override
Expand Down Expand Up @@ -439,14 +442,18 @@ public void dropView(ConnectorSession session, SchemaTableName schemaViewName)
@Override
public List<SchemaTableName> listViews(ConnectorSession session, Optional<String> namespace)
{
// Filter on PRESTO_VIEW_COMMENT to distinguish from materialized views
return listNamespaces(session, namespace).stream()
.flatMap(schema ->
metastore.getTablesWithParameter(schema, TABLE_COMMENT, PRESTO_VIEW_COMMENT).stream()
.map(table -> new SchemaTableName(schema, table)))
.flatMap(this::listViews)
.collect(toImmutableList());
}

private Stream<SchemaTableName> listViews(String schema)
{
// Filter on PRESTO_VIEW_COMMENT to distinguish from materialized views
return metastore.getTablesWithParameter(schema, TABLE_COMMENT, PRESTO_VIEW_COMMENT).stream()
.map(table -> new SchemaTableName(schema, table));
}

@Override
public Map<SchemaTableName, ConnectorViewDefinition> getViews(ConnectorSession session, Optional<String> namespace)
{
Expand Down
6 changes: 5 additions & 1 deletion plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/RaptorMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,11 @@ public ConnectorTableMetadata getTableMetadata(ConnectorSession session, Connect
@Override
public List<SchemaTableName> listTables(ConnectorSession session, Optional<String> schemaName)
{
return dao.listTables(schemaName.orElse(null));
// Deduplicate with set because state may change concurrently
return ImmutableSet.<SchemaTableName>builder()
.addAll(dao.listTables(schemaName.orElse(null)))
.addAll(listViews(session, schemaName))
.build().asList();
}

@Override
Expand Down
112 changes: 110 additions & 2 deletions testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ public void testView()
return;
}

@Language("SQL") String query = "SELECT orderkey, orderstatus, totalprice / 2 half FROM orders";
@Language("SQL") String query = "SELECT orderkey, orderstatus, (totalprice / 2) half FROM orders";

String catalogName = getSession().getCatalog().orElseThrow();
String schemaName = getSession().getSchema().orElseThrow();
Expand All @@ -807,6 +807,7 @@ public void testView()
assertUpdate("CREATE VIEW " + testViewWithComment + " COMMENT 'orders' AS SELECT 123 x");
assertUpdate("CREATE OR REPLACE VIEW " + testViewWithComment + " COMMENT 'orders' AS " + query);

// verify comment
MaterializedResult materializedRows = computeActual("SHOW CREATE VIEW " + testViewWithComment);
assertThat((String) materializedRows.getOnlyValue()).contains("COMMENT 'orders'");
assertThat(query(
Expand All @@ -816,6 +817,7 @@ public void testView()
.skippingTypesCheck()
.containsAll("VALUES ('" + testView + "', null), ('" + testViewWithComment + "', 'orders')");

// reading
assertQuery("SELECT * FROM " + testView, query);
assertQuery("SELECT * FROM " + testViewWithComment, query);

Expand All @@ -828,8 +830,114 @@ public void testView()
String name = format("%s.%s." + testView, catalogName, schemaName);
assertQuery("SELECT * FROM " + name, query);

assertUpdate("DROP VIEW " + testView);
assertUpdate("DROP VIEW " + testViewWithComment);

// information_schema.views without table_name filter
assertThat(query(
"SELECT table_name, regexp_replace(view_definition, '\\s', '') FROM information_schema.views " +
"WHERE table_schema = '" + schemaName + "'"))
.skippingTypesCheck()
.containsAll("VALUES ('" + testView + "', '" + query.replaceAll("\\s", "") + "')");
// information_schema.views with table_name filter
assertQuery(
"SELECT table_name, regexp_replace(view_definition, '\\s', '') FROM information_schema.views " +
"WHERE table_schema = '" + schemaName + "' and table_name = '" + testView + "'",
"VALUES ('" + testView + "', '" + query.replaceAll("\\s", "") + "')");

// table listing
assertThat(query("SHOW TABLES"))
.skippingTypesCheck()
.containsAll("VALUES '" + testView + "'");
// information_schema.tables without table_name filter
assertThat(query(
"SELECT table_name, table_type FROM information_schema.tables " +
"WHERE table_schema = '" + schemaName + "'"))
.skippingTypesCheck()
.containsAll("VALUES ('" + testView + "', 'VIEW')");
// information_schema.tables with table_name filter
assertQuery(
"SELECT table_name, table_type FROM information_schema.tables " +
"WHERE table_schema = '" + schemaName + "' and table_name = '" + testView + "'",
"VALUES ('" + testView + "', 'VIEW')");

// system.jdbc.tables without filter
assertThat(query("SELECT table_schem, table_name, table_type FROM system.jdbc.tables"))
.skippingTypesCheck()
.containsAll("VALUES ('" + schemaName + "', '" + testView + "', 'VIEW')");

// system.jdbc.tables with table prefix filter
assertQuery(
"SELECT table_schem, table_name, table_type " +
"FROM system.jdbc.tables " +
"WHERE table_cat = '" + catalogName + "' AND " +
"table_schem = '" + schemaName + "' AND " +
"table_name = '" + testView + "'",
"VALUES ('" + schemaName + "', '" + testView + "', 'VIEW')");

// column listing
assertThat(query("SHOW COLUMNS FROM " + testView))
.projected(0) // column types can very between connectors
.skippingTypesCheck()
.matches("VALUES 'orderkey', 'orderstatus', 'half'");

assertThat(query("DESCRIBE " + testView))
.projected(0) // column types can very between connectors
.skippingTypesCheck()
.matches("VALUES 'orderkey', 'orderstatus', 'half'");

// information_schema.columns without table_name filter
assertThat(query(
"SELECT table_name, column_name " +
"FROM information_schema.columns " +
"WHERE table_schema = '" + schemaName + "'"))
.skippingTypesCheck()
.containsAll(
"SELECT * FROM (VALUES '" + testView + "') " +
"CROSS JOIN UNNEST(ARRAY['orderkey', 'orderstatus', 'half'])");

// information_schema.columns with table_name filter
assertThat(query(
"SELECT table_name, column_name " +
"FROM information_schema.columns " +
"WHERE table_schema = '" + schemaName + "' and table_name = '" + testView + "'"))
.skippingTypesCheck()
.containsAll(
"SELECT * FROM (VALUES '" + testView + "') " +
"CROSS JOIN UNNEST(ARRAY['orderkey', 'orderstatus', 'half'])");

// view-specific listings
assertThat(query("SELECT table_name FROM information_schema.views WHERE table_schema = '" + schemaName + "'"))
.skippingTypesCheck()
.containsAll("VALUES '" + testView + "'");

// system.jdbc.columns without filter
assertThat(query("SELECT table_schem, table_name, column_name FROM system.jdbc.columns"))
.skippingTypesCheck()
.containsAll(
"SELECT * FROM (VALUES ('" + schemaName + "', '" + testView + "')) " +
"CROSS JOIN UNNEST(ARRAY['orderkey', 'orderstatus', 'half'])");

// system.jdbc.columns with schema filter
assertThat(query(
"SELECT table_schem, table_name, column_name " +
"FROM system.jdbc.columns " +
"WHERE table_schem LIKE '%" + schemaName + "%'"))
.skippingTypesCheck()
.containsAll(
"SELECT * FROM (VALUES ('" + schemaName + "', '" + testView + "')) " +
"CROSS JOIN UNNEST(ARRAY['orderkey', 'orderstatus', 'half'])");

// system.jdbc.columns with table filter
assertThat(query(
"SELECT table_schem, table_name, column_name " +
"FROM system.jdbc.columns " +
"WHERE table_name LIKE '%" + testView + "%'"))
.skippingTypesCheck()
.containsAll(
"SELECT * FROM (VALUES ('" + schemaName + "', '" + testView + "')) " +
"CROSS JOIN UNNEST(ARRAY['orderkey', 'orderstatus', 'half'])");

assertUpdate("DROP VIEW " + testView);
}

@Test
Expand Down
24 changes: 13 additions & 11 deletions testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,29 +624,31 @@ public void testMaterializedView()
checkInformationSchemaViewsForMaterializedView(view.getSchemaName(), view.getObjectName());

// system.jdbc.columns without filter
@Language("SQL") String expectedValues = "VALUES ('" + view.getSchemaName() + "', '" + view.getObjectName() + "', 'nationkey'), " +
"('" + view.getSchemaName() + "', '" + view.getObjectName() + "', 'name'), " +
"('" + view.getSchemaName() + "', '" + view.getObjectName() + "', 'regionkey'), " +
"('" + view.getSchemaName() + "', '" + view.getObjectName() + "', 'comment')";
assertThat(query(
"SELECT table_schem, table_name, column_name FROM system.jdbc.columns"))
assertThat(query("SELECT table_schem, table_name, column_name FROM system.jdbc.columns"))
.skippingTypesCheck()
.containsAll(expectedValues);
.containsAll(
"SELECT * FROM (VALUES ('" + view.getSchemaName() + "', '" + view.getObjectName() + "')) " +
"CROSS JOIN UNNEST(ARRAY['nationkey', 'name', 'regionkey', 'comment'])");

// system.jdbc.columns with schema filter
assertThat(query(
"SELECT table_schem, table_name, column_name " +
"FROM system.jdbc.columns " +
"WHERE table_schem LIKE '%" + view.getSchemaName() + "%'"))
.skippingTypesCheck()
.containsAll(expectedValues);
.containsAll(
"SELECT * FROM (VALUES ('" + view.getSchemaName() + "', '" + view.getObjectName() + "')) " +
"CROSS JOIN UNNEST(ARRAY['nationkey', 'name', 'regionkey', 'comment'])");

// system.jdbc.columns with table filter
assertQuery(
assertThat(query(
"SELECT table_schem, table_name, column_name " +
"FROM system.jdbc.columns " +
"WHERE table_name LIKE '%" + view.getObjectName() + "%'",
expectedValues);
"WHERE table_name LIKE '%" + view.getObjectName() + "%'"))
.skippingTypesCheck()
.containsAll(
"SELECT * FROM (VALUES ('" + view.getSchemaName() + "', '" + view.getObjectName() + "')) " +
"CROSS JOIN UNNEST(ARRAY['nationkey', 'name', 'regionkey', 'comment'])");

// details
assertThat(((String) computeScalar("SHOW CREATE MATERIALIZED VIEW " + view.getObjectName())))
Expand Down