information_schema.views fixes for materialized views #17444
Conversation
findepi
requested review from
martint,
losipiuk,
huberty89,
raunaqmorarka,
sopel39,
kokosing and
hashhar
testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java
Show resolved
Hide resolved
github-actions
bot
added
hive
testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java
Show resolved
Hide resolved
|
(just rebased) |
| @@ -337,14 +337,6 @@ public void testShowCreateTable() | |||
| ")\\E"); | |||
There was a problem hiding this comment.
Despite a materialized view having
VIEW
table_typeininformation_schema.tables, they are not supposed to
show up inviews.
So where they should appear? Is this part of SQL standard?
There was a problem hiding this comment.
The commit message is wrong, will fix it.
we have system.metadata.materialized_views for them
There was a problem hiding this comment.
Is this part of SQL standard?
afaik MVs are extension
The test expected that materialized views show up in `information_schama.views`. Materialized views are reported with `table_type=BASE TABLE` in `information_schema.tables` and are not supposed to show up in `information_schema.views`.
Test system.metadata.materialized_view when testing metadata listings (eg information_schema.views).
|
rebased to be on top of #17127 and fixed first commit message per #17444 (comment) |
findepi
force-pushed
the
findepi/mvlist
branch
2 times, most recently
from
18b5a1a to
1b6d88e
Compare
Fix value in `table_type` column of `information_schema.tables` for - materialized views when Iceberg connector is backed by Glue catalog - materialized views in Hive connector (when `HiveMaterializedViewMetadata` extension is used).
|
CI #16277 |
| @@ -556,6 +556,20 @@ public void testMaterializedView() | |||
| "FROM\n" + | |||
| " nation"); | |||
|
|
|||
| // information_schema.tables (no filtering on table_name so that ConnectorMetadata.listViews is exercised) | |||
| assertThat(query("SELECT table_name, table_type FROM information_schema.tables WHERE table_schema = '" + schemaName + "'")) | |||
| .containsAll("VALUES (VARCHAR '" + viewName + "', VARCHAR 'BASE TABLE')"); | |||
There was a problem hiding this comment.
why view is marked as base table?
| .returnsEmptyResult(); | ||
|
|
||
| // materialized view-specific listings | ||
| assertThat(query("SELECT name FROM system.metadata.materialized_views WHERE catalog_name = '" + catalogName + "' AND schema_name = '" + schemaName + "'")) |
There was a problem hiding this comment.
are there any other columns that we should test?
There was a problem hiding this comment.
i chose name only because i focus on the listing ("is or isn't" there). we can revisit as needed
| .containsAll("VALUES (VARCHAR '" + viewName + "', VARCHAR 'BASE TABLE')"); | ||
|
|
||
| // information_schema.views | ||
| assertThat(computeActual("SELECT table_name FROM information_schema.views WHERE table_schema = '" + schemaName + "'").getOnlyColumnAsSet()) |
There was a problem hiding this comment.
are there any other columns that we should test?
|
Does this need release notes? @findepi |
findepi
added
the
no-release-notes
|
i don't think so, Cole |
No description provided.