Skip to content

Commit ef76b29

Browse files
piotrrzyskopiotrrzysko
committed
Skip underlying query analysis if MV is fresh
Analyzing the underlying query can be costly in some scenarios, especially when it references many tables. An additional benefit of skipping this analysis is improved availability — MV data sources don’t need to be reachable when accessing the MV.
1 parent 1202373 commit ef76b29

File tree

3 files changed

+106
-38
lines changed

3 files changed

+106
-38
lines changed

‎core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2610,19 +2610,6 @@ private Scope createScopeForView(
26102610
throw semanticException(VIEW_IS_RECURSIVE, table, "View is recursive");
26112611
}
26122612

2613-
Query query = parseView(originalSql, name, table);
2614-
2615-
if (!query.getFunctions().isEmpty()) {
2616-
throw semanticException(NOT_SUPPORTED, table, "View contains inline function: %s", name);
2617-
}
2618-
2619-
analysis.registerTableForView(table, name, isMaterializedView);
2620-
RelationType descriptor = analyzeView(query, name, catalog, schema, owner, path, table);
2621-
analysis.unregisterTableForView();
2622-
2623-
checkViewStaleness(columns, descriptor.getVisibleFields(), name, table)
2624-
.ifPresent(explanation -> { throw semanticException(VIEW_IS_STALE, table, "View '%s' is stale or in invalid state: %s", name, explanation); });
2625-
26262613
// Derive the type of the view from the stored definition, not from the analysis of the underlying query.
26272614
// This is needed in case the underlying table(s) changed and the query in the view now produces types that
26282615
// are implicitly coercible to the declared view types.
@@ -2642,6 +2629,19 @@ private Scope createScopeForView(
26422629
analysis.setMaterializedViewStorageTableFields(table, storageTableFields);
26432630
}
26442631
else {
2632+
Query query = parseView(originalSql, name, table);
2633+
2634+
if (!query.getFunctions().isEmpty()) {
2635+
throw semanticException(NOT_SUPPORTED, table, "View contains inline function: %s", name);
2636+
}
2637+
2638+
analysis.registerTableForView(table, name, isMaterializedView);
2639+
RelationType descriptor = analyzeView(query, name, catalog, schema, owner, path, table);
2640+
analysis.unregisterTableForView();
2641+
2642+
checkViewStaleness(columns, descriptor.getVisibleFields(), name, table)
2643+
.ifPresent(explanation -> { throw semanticException(VIEW_IS_STALE, table, "View '%s' is stale or in invalid state: %s", name, explanation); });
2644+
26452645
analysis.registerNamedQuery(table, query);
26462646
}
26472647

‎core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java‎

Lines changed: 89 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@
208208
import static io.trino.testing.TestingAccessControlManager.TestingPrivilegeType.SELECT_COLUMN;
209209
import static io.trino.testing.TestingAccessControlManager.privilege;
210210
import static io.trino.testing.TestingEventListenerManager.emptyEventListenerManager;
211+
import static io.trino.testing.TestingMetadata.STALE_MV_STALENESS;
211212
import static io.trino.testing.TestingSession.testSessionBuilder;
212213
import static io.trino.testing.TransactionBuilder.transaction;
213214
import static io.trino.testing.assertions.TrinoExceptionAssert.assertTrinoExceptionThrownBy;
@@ -5797,6 +5798,22 @@ public void testRowPatternCountFunction()
57975798
public void testAnalyzeFreshMaterializedView()
57985799
{
57995800
analyze("SELECT * FROM fresh_materialized_view");
5801+
analyze("SELECT * FROM fresh_materialized_view_non_existent_table");
5802+
assertFails("REFRESH MATERIALIZED VIEW fresh_materialized_view_non_existent_table")
5803+
.hasErrorCode(TABLE_NOT_FOUND)
5804+
.hasMessage("line 1:18: Table 'tpch.s1.non_existent_table' does not exist");
5805+
}
5806+
5807+
@Test
5808+
public void testAnalyzeStaleMaterializedView()
5809+
{
5810+
analyze("SELECT * FROM stale_materialized_view");
5811+
assertFails("SELECT * FROM stale_materialized_view_non_existent_table")
5812+
.hasErrorCode(INVALID_VIEW)
5813+
.hasMessage("line 1:15: Failed analyzing stored view 'tpch.s1.stale_materialized_view_non_existent_table': line 1:18: Table 'tpch.s1.non_existent_table' does not exist");
5814+
assertFails("REFRESH MATERIALIZED VIEW stale_materialized_view_non_existent_table")
5815+
.hasErrorCode(TABLE_NOT_FOUND)
5816+
.hasMessage("line 1:18: Table 'tpch.s1.non_existent_table' does not exist");
58005817
}
58015818

58025819
@Test
@@ -5832,6 +5849,23 @@ public void testAnalyzeMaterializedViewWithAccessControl()
58325849
accessControlManager)
58335850
.hasErrorCode(PERMISSION_DENIED)
58345851
.hasMessage("Access Denied: Cannot select from columns [a, b] in table or view tpch.s1.fresh_materialized_view");
5852+
accessControlManager.reset();
5853+
5854+
// Deny access to the table referenced by the underlying query
5855+
accessControlManager.denyIdentityTable((_, table) -> !"t1".equals(table));
5856+
// When an MV is fresh or within the grace period, analysis of the underlying query is not performed,
5857+
// so access control checks on the tables referenced by the query are not executed.
5858+
analyze(CLIENT_SESSION, "SELECT * FROM fresh_materialized_view", accessControlManager);
5859+
assertFails(CLIENT_SESSION, "REFRESH MATERIALIZED VIEW fresh_materialized_view", accessControlManager)
5860+
.hasErrorCode(PERMISSION_DENIED)
5861+
.hasMessage("Access Denied: Cannot select from columns [a, b] in table or view tpch.s1.t1");
5862+
// Now we check that when the MV is stale, access to the underlying tables is checked.
5863+
assertFails(CLIENT_SESSION, "SELECT * FROM stale_materialized_view", accessControlManager)
5864+
.hasErrorCode(PERMISSION_DENIED)
5865+
.hasMessage("Access Denied: View owner does not have sufficient privileges: View owner 'some user' cannot create view that selects from tpch.s1.t1");
5866+
assertFails(CLIENT_SESSION, "REFRESH MATERIALIZED VIEW stale_materialized_view", accessControlManager)
5867+
.hasErrorCode(PERMISSION_DENIED)
5868+
.hasMessage("Access Denied: Cannot select from columns [a, b] in table or view tpch.s1.t1");
58355869
}
58365870

58375871
@Test
@@ -8074,27 +8108,70 @@ public void setup()
80748108
ImmutableList.of(new ColumnMetadata("a", BIGINT))),
80758109
FAIL));
80768110

8111+
MaterializedViewDefinition materializedView = new MaterializedViewDefinition(
8112+
"SELECT a, b FROM t1",
8113+
Optional.of(TPCH_CATALOG),
8114+
Optional.of("s1"),
8115+
ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty()), new ViewColumn("b", BIGINT.getTypeId(), Optional.empty())),
8116+
Optional.of(STALE_MV_STALENESS.minusHours(1)), // Minus 1 hour to make MVs not marked as fresh become stale immediately. This doesn’t affect those marked as fresh.
8117+
Optional.empty(),
8118+
Optional.empty(),
8119+
Identity.ofUser("some user"),
8120+
ImmutableList.of(),
8121+
// t3 has a, b column and hidden column x
8122+
Optional.of(new CatalogSchemaTableName(TPCH_CATALOG, "s1", "t3")));
8123+
80778124
QualifiedObjectName freshMaterializedView = new QualifiedObjectName(TPCH_CATALOG, "s1", "fresh_materialized_view");
80788125
inSetupTransaction(session -> metadata.createMaterializedView(
80798126
session,
80808127
freshMaterializedView,
8081-
new MaterializedViewDefinition(
8082-
"SELECT a, b FROM t1",
8083-
Optional.of(TPCH_CATALOG),
8084-
Optional.of("s1"),
8085-
ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty()), new ViewColumn("b", BIGINT.getTypeId(), Optional.empty())),
8086-
Optional.empty(),
8087-
Optional.empty(),
8088-
Optional.empty(),
8089-
Identity.ofUser("some user"),
8090-
ImmutableList.of(),
8091-
// t3 has a, b column and hidden column x
8092-
Optional.of(new CatalogSchemaTableName(TPCH_CATALOG, "s1", "t3"))),
8128+
materializedView,
80938129
ImmutableMap.of(),
80948130
false,
80958131
false));
80968132
testingConnectorMetadata.markMaterializedViewIsFresh(freshMaterializedView.asSchemaTableName());
80978133

8134+
QualifiedObjectName staleMaterializedView = new QualifiedObjectName(TPCH_CATALOG, "s1", "stale_materialized_view");
8135+
inSetupTransaction(session -> metadata.createMaterializedView(
8136+
session,
8137+
staleMaterializedView,
8138+
materializedView,
8139+
ImmutableMap.of(),
8140+
false,
8141+
false));
8142+
8143+
MaterializedViewDefinition materializedViewNonExistentTable = new MaterializedViewDefinition(
8144+
"SELECT a, b FROM non_existent_table",
8145+
Optional.of(TPCH_CATALOG),
8146+
Optional.of("s1"),
8147+
ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty()), new ViewColumn("b", BIGINT.getTypeId(), Optional.empty())),
8148+
Optional.of(STALE_MV_STALENESS.minusHours(1)), // Minus 1 hour to make MVs not marked as fresh become stale immediately. This doesn’t affect those marked as fresh.
8149+
Optional.empty(),
8150+
Optional.empty(),
8151+
Identity.ofUser("some user"),
8152+
ImmutableList.of(),
8153+
// t3 has a, b column and hidden column x
8154+
Optional.of(new CatalogSchemaTableName(TPCH_CATALOG, "s1", "t3")));
8155+
8156+
QualifiedObjectName freshMaterializedViewNonExistentTable = new QualifiedObjectName(TPCH_CATALOG, "s1", "fresh_materialized_view_non_existent_table");
8157+
inSetupTransaction(session -> metadata.createMaterializedView(
8158+
session,
8159+
freshMaterializedViewNonExistentTable,
8160+
materializedViewNonExistentTable,
8161+
ImmutableMap.of(),
8162+
false,
8163+
false));
8164+
testingConnectorMetadata.markMaterializedViewIsFresh(freshMaterializedViewNonExistentTable.asSchemaTableName());
8165+
8166+
QualifiedObjectName staleMaterializedViewNonExistentTable = new QualifiedObjectName(TPCH_CATALOG, "s1", "stale_materialized_view_non_existent_table");
8167+
inSetupTransaction(session -> metadata.createMaterializedView(
8168+
session,
8169+
staleMaterializedViewNonExistentTable,
8170+
materializedViewNonExistentTable,
8171+
ImmutableMap.of(),
8172+
false,
8173+
false));
8174+
80988175
QualifiedObjectName freshMaterializedViewMismatchedColumnCount = new QualifiedObjectName(TPCH_CATALOG, "s1", "fresh_materialized_view_mismatched_column_count");
80998176
inSetupTransaction(session -> metadata.createMaterializedView(
81008177
session,

‎testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,8 @@ public void testReferencedTablesWithViews()
510510
.hasNoTableReferences();
511511
}
512512

513+
// Currently, the storage table for a materialized view is not included anywhere in the set of `tables` in the query event.
514+
// See for more details: https://github.com/trinodb/trino/pull/18871#discussion_r1412247513
513515
@Test
514516
public void testReferencedTablesWithMaterializedViewsStale()
515517
throws Exception
@@ -540,8 +542,6 @@ public void testReferencedTablesWithMaterializedViewsStale()
540542
.hasViewText("SELECT nationkey AS test_column FROM tpch.tiny.nation");
541543
}
542544

543-
// Currently, the storage table for a materialized view is not included anywhere in the set of `tables` in the query event.
544-
// See for more details: https://github.com/trinodb/trino/pull/18871#discussion_r1412247513
545545
@Test
546546
public void testReferencedTablesWithMaterializedViewsFresh()
547547
throws Exception
@@ -551,17 +551,8 @@ public void testReferencedTablesWithMaterializedViewsFresh()
551551
QueryCompletedEvent event = queryEvents.getQueryCompletedEvent();
552552

553553
List<TableInfo> tables = event.getMetadata().getTables();
554-
assertThat(tables).hasSize(2);
555-
TableInfo table = tables.get(0);
556-
assertThat(table)
557-
.hasCatalogSchemaTable("tpch", "tiny", "nation")
558-
.hasAuthorization("alice")
559-
.isNotDirectlyReferenced()
560-
.hasColumnsWithoutMasking("nationkey", "regionkey", "name", "comment")
561-
.hasNoRowFilters()
562-
.hasTableReferencesSatisfying(tableRef -> assertThat(tableRef).asMaterializedViewInfo().hasCatalogSchemaView("mock", "default", "test_materialized_view_fresh"));
563-
564-
table = tables.get(1);
554+
assertThat(tables).hasSize(1);
555+
TableInfo table = tables.getFirst();
565556
assertThat(table)
566557
.hasCatalogSchemaTable("mock", "default", "test_materialized_view_fresh")
567558
.hasAuthorization("user")

0 commit comments

Comments
 (0)