Skip to content

fix: Check MV base table permission when stitching disabled #26580

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
zation99 merged 1 commit into from
Nov 14, 2025
Merged

fix: Check MV base table permission when stitching disabled #26580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
71 changes: 69 additions & 2 deletions presto-hive/src/test/java/com/facebook/presto/hive/TestHiveIntegrationSmokeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@
import static com.facebook.presto.SystemSessionProperties.JOIN_DISTRIBUTION_TYPE;
import static com.facebook.presto.SystemSessionProperties.JOIN_REORDERING_STRATEGY;
import static com.facebook.presto.SystemSessionProperties.LOG_INVOKED_FUNCTION_NAMES_ENABLED;
import static com.facebook.presto.SystemSessionProperties.MATERIALIZED_VIEW_ALLOW_FULL_REFRESH_ENABLED;
import static com.facebook.presto.SystemSessionProperties.MATERIALIZED_VIEW_DATA_CONSISTENCY_ENABLED;
import static com.facebook.presto.SystemSessionProperties.PARTIAL_MERGE_PUSHDOWN_STRATEGY;
import static com.facebook.presto.SystemSessionProperties.PARTITIONING_PROVIDER_CATALOG;
import static com.facebook.presto.common.predicate.Marker.Bound.EXACTLY;
Expand Down Expand Up @@ -6111,8 +6113,6 @@ public void testRefreshMaterializedView()
@Test
public void testAutoRefreshMaterializedViewFailsWithoutFlag()
{
QueryRunner queryRunner = getQueryRunner();

computeActual("CREATE TABLE test_orders_no_flag WITH (partitioned_by = ARRAY['orderstatus']) " +
"AS SELECT orderkey, totalprice, orderstatus FROM orders WHERE orderkey < 100");
computeActual(
Expand All @@ -6127,6 +6127,73 @@ public void testAutoRefreshMaterializedViewFailsWithoutFlag()
computeActual("DROP TABLE test_orders_no_flag");
}

@Test
public void testMaterializedViewBaseTableDropped()
{
Session stitchingEnabledSession = Session.builder(getSession())
.setSystemProperty(MATERIALIZED_VIEW_ALLOW_FULL_REFRESH_ENABLED, "true")
.build();
Session stichingDisabledSession = Session.builder(getSession())
.setSystemProperty(MATERIALIZED_VIEW_DATA_CONSISTENCY_ENABLED, "false")
.build();

assertUpdate("CREATE TABLE drop_table_test (id BIGINT, partkey VARCHAR) " +
"WITH (partitioned_by=ARRAY['partkey'])");
assertUpdate("INSERT INTO drop_table_test VALUES (1, 'p1'), (2, 'p2')", 2);

assertUpdate("CREATE MATERIALIZED VIEW mv_drop " +
"with (partitioned_by=ARRAY['partkey']) " +
"AS SELECT id, partkey FROM drop_table_test");

assertUpdate(stitchingEnabledSession, "REFRESH MATERIALIZED VIEW mv_drop", 2);

assertUpdate("DROP TABLE drop_table_test");

assertQueryFails(stitchingEnabledSession, "SELECT COUNT(*) FROM mv_drop",
".*Table .* not found.*");
assertQueryFails(stichingDisabledSession, "SELECT * FROM mv_drop ORDER BY id",
".*Table .* does not exist.*");

assertQueryFails("REFRESH MATERIALIZED VIEW mv_drop",
".*Table .* not found.*");

computeActual("DROP MATERIALIZED VIEW mv_drop");
}

@Test
public void testMaterializedViewBaseTableRenamed()
{
Session stitchingEnabledSession = Session.builder(getSession())
.setSystemProperty(MATERIALIZED_VIEW_ALLOW_FULL_REFRESH_ENABLED, "true")
.build();
Session stichingDisabledSession = Session.builder(getSession())
.setSystemProperty(MATERIALIZED_VIEW_DATA_CONSISTENCY_ENABLED, "false")
.build();

assertUpdate("CREATE TABLE rename_table_test (id BIGINT, partkey VARCHAR) " +
"WITH (partitioned_by=ARRAY['partkey'])");
assertUpdate("INSERT INTO rename_table_test VALUES (1, 'p1'), (2, 'p2')", 2);

assertUpdate("CREATE MATERIALIZED VIEW mv_rename_test " +
"with (partitioned_by=ARRAY['partkey']) " +
"AS SELECT id, partkey FROM rename_table_test");

assertUpdate(stitchingEnabledSession, "REFRESH MATERIALIZED VIEW mv_rename_test", 2);

assertUpdate("ALTER TABLE rename_table_test RENAME TO rename_table_test_new");

assertQueryFails(stitchingEnabledSession, "SELECT COUNT(*) FROM mv_rename_test",
".*Table .* not found.*");
assertQueryFails(stichingDisabledSession, "SELECT * FROM mv_rename_test ORDER BY id",
".*Table .* does not exist.*");

assertQueryFails("REFRESH MATERIALIZED VIEW mv_rename_test",
".*Table .* not found.*");

computeActual("DROP TABLE rename_table_test_new");
computeActual("DROP MATERIALIZED VIEW mv_rename_test");
}

@Test
public void testAlphaFormatDdl()
{
Expand Down
82 changes: 59 additions & 23 deletions ...o-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;

import static com.facebook.presto.SystemSessionProperties.CONSIDER_QUERY_FILTERS_FOR_MATERIALIZED_VIEW_PARTITIONS;
import static com.facebook.presto.SystemSessionProperties.JOIN_DISTRIBUTION_TYPE;
import static com.facebook.presto.SystemSessionProperties.JOIN_REORDERING_STRATEGY;
import static com.facebook.presto.SystemSessionProperties.MATERIALIZED_VIEW_DATA_CONSISTENCY_ENABLED;
import static com.facebook.presto.SystemSessionProperties.PREFER_PARTIAL_AGGREGATION;
import static com.facebook.presto.SystemSessionProperties.QUERY_OPTIMIZATION_WITH_MATERIALIZED_VIEW_ENABLED;
import static com.facebook.presto.SystemSessionProperties.SIMPLIFY_PLAN_WITH_EMPTY_INPUT;
Expand Down Expand Up @@ -2593,12 +2593,25 @@ public void testInsertBySelectingFromMaterializedView()
public void testMaterializedViewQueryAccessControl()
{
QueryRunner queryRunner = getQueryRunner();
Session invokerSession = Session.builder(getSession())

Session invokerStichingSession = Session.builder(getSession())
.setIdentity(new Identity("test_view_invoker", Optional.empty()))
.setCatalog(getSession().getCatalog().get())
.setSchema(getSession().getSchema().get())
.setSystemProperty(QUERY_OPTIMIZATION_WITH_MATERIALIZED_VIEW_ENABLED, "true")
.build();

/* Non-stitching session test is needed as the query is not rewritten
with base table. In this case the analyzer should process the materialized view
definition sql to check all the base tables permissions.
*/
Session invokerNonStichingSession = Session.builder(getSession())
.setIdentity(new Identity("test_view_invoker2", Optional.empty()))
.setCatalog(getSession().getCatalog().get())
.setSchema(getSession().getSchema().get())
.setSystemProperty(MATERIALIZED_VIEW_DATA_CONSISTENCY_ENABLED, "false")
.build();

Session ownerSession = getSession();

queryRunner.execute(
Expand All @@ -2612,37 +2625,60 @@ public void testMaterializedViewQueryAccessControl()
"AS SELECT SUM(totalprice) AS totalprice, orderstatus FROM test_orders_base GROUP BY orderstatus");
setReferencedMaterializedViews((DistributedQueryRunner) getQueryRunner(), "test_orders_base", ImmutableList.of("test_orders_view"));

Consumer<String> testQueryWithDeniedPrivilege = query -> {
// Verify checking the base table instead of the materialized view for SELECT permission
try {
// Check for both the direct materialized view query and the base table query optimization with materialized view
// for direct materialized view query, check when stitching is enabled/disabled
String queryMaterializedView = "SELECT totalprice, orderstatus FROM test_orders_view";
String queryBaseTable = "SELECT SUM(totalprice) AS totalprice, orderstatus FROM test_orders_base GROUP BY orderstatus";

assertAccessDenied(
invokerSession,
query,
invokerNonStichingSession,
queryBaseTable,
"Cannot select from columns \\[.*\\] in table .*test_orders_base.*",
privilege(invokerSession.getUser(), "test_orders_base", SELECT_COLUMN));
assertAccessAllowed(
invokerSession,
query,
privilege(invokerSession.getUser(), "test_orders_view", SELECT_COLUMN));
};
privilege(invokerNonStichingSession.getUser(), "test_orders_base", SELECT_COLUMN));

try {
// Check for both the direct materialized view query and the base table query optimization with materialized view
String directMaterializedViewQuery = "SELECT totalprice, orderstatus FROM test_orders_view";
String queryWithMaterializedViewOptimization = "SELECT SUM(totalprice) AS totalprice, orderstatus FROM test_orders_base GROUP BY orderstatus";
assertAccessDenied(
invokerStichingSession,
queryBaseTable,
"Cannot select from columns \\[.*\\] in table .*test_orders_base.*",
privilege(invokerStichingSession.getUser(), "test_orders_base", SELECT_COLUMN));

assertAccessAllowed(
invokerStichingSession,
queryMaterializedView,
privilege(invokerStichingSession.getUser(), "test_orders_view", SELECT_COLUMN));

// Test when the materialized view is not materialized yet
testQueryWithDeniedPrivilege.accept(directMaterializedViewQuery);
testQueryWithDeniedPrivilege.accept(queryWithMaterializedViewOptimization);
assertAccessDenied(
invokerNonStichingSession,
queryMaterializedView,
"Cannot select from columns \\[.*\\] in table .*test_orders_base.*",
privilege(invokerNonStichingSession.getUser(), "test_orders_base", SELECT_COLUMN));

// Test when the materialized view is partially materialized
queryRunner.execute(ownerSession, "REFRESH MATERIALIZED VIEW test_orders_view WHERE orderstatus = 'F'");
testQueryWithDeniedPrivilege.accept(directMaterializedViewQuery);
testQueryWithDeniedPrivilege.accept(queryWithMaterializedViewOptimization);
assertAccessAllowed(
invokerStichingSession,
queryMaterializedView,
privilege(invokerStichingSession.getUser(), "test_orders_view", SELECT_COLUMN));

assertAccessDenied(
invokerNonStichingSession,
queryMaterializedView,
"Cannot select from columns \\[.*\\] in table .*test_orders_base.*",
privilege(invokerNonStichingSession.getUser(), "test_orders_base", SELECT_COLUMN));

// Test when the materialized view is fully materialized
queryRunner.execute(ownerSession, "REFRESH MATERIALIZED VIEW test_orders_view WHERE orderstatus <> 'F'");
testQueryWithDeniedPrivilege.accept(directMaterializedViewQuery);
testQueryWithDeniedPrivilege.accept(queryWithMaterializedViewOptimization);
assertAccessAllowed(
invokerStichingSession,
queryMaterializedView,
privilege(invokerStichingSession.getUser(), "test_orders_view", SELECT_COLUMN));

assertAccessDenied(
invokerNonStichingSession,
queryMaterializedView,
"Cannot select from columns \\[.*\\] in table .*test_orders_base.*",
privilege(invokerNonStichingSession.getUser(), "test_orders_base", SELECT_COLUMN));
}
finally {
queryRunner.execute(ownerSession, "DROP MATERIALIZED VIEW test_orders_view");
Expand Down
29 changes: 22 additions & 7 deletions presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2089,15 +2089,30 @@ protected Scope visitTable(Table table, Optional<Scope> scope)
optionalMaterializedView.get().getTable());
}
Statement statement = analysis.getStatement();
if (isMaterializedViewDataConsistencyEnabled(session) && optionalMaterializedView.isPresent() && statement instanceof Query) {
// When the materialized view has already been expanded, do not process it. Just use it as a table.
MaterializedViewAnalysisState materializedViewAnalysisState = analysis.getMaterializedViewAnalysisState(table);
if (optionalMaterializedView.isPresent() && statement instanceof Query) {
if (isMaterializedViewDataConsistencyEnabled(session)) {
// When the materialized view has already been expanded, do not process it. Just use it as a table.
MaterializedViewAnalysisState materializedViewAnalysisState = analysis.getMaterializedViewAnalysisState(table);

if (materializedViewAnalysisState.isNotVisited()) {
return processMaterializedView(table, name, scope, optionalMaterializedView.get());
if (materializedViewAnalysisState.isNotVisited()) {
return processMaterializedView(table, name, scope, optionalMaterializedView.get());
}
if (materializedViewAnalysisState.isVisited()) {
throw new SemanticException(MATERIALIZED_VIEW_IS_RECURSIVE, table, "Materialized view is recursive");
}
}
if (materializedViewAnalysisState.isVisited()) {
throw new SemanticException(MATERIALIZED_VIEW_IS_RECURSIVE, table, "Materialized view is recursive");
else {
// when stitching is not enabled, still check permission of each base table
MaterializedViewDefinition materializedViewDefinition = optionalMaterializedView.get();
analysis.getAccessControlReferences().addMaterializedViewDefinitionReference(name, materializedViewDefinition);

Query viewQuery = (Query) sqlParser.createStatement(
materializedViewDefinition.getOriginalSql(),
createParsingOptions(session, warningCollector));

analysis.registerMaterializedViewForAnalysis(name, table, materializedViewDefinition.getOriginalSql());
process(viewQuery, scope);
Copy link
Copy Markdown
Member

@hantangwangd hantangwangd Nov 13, 2025

Choose a reason for hiding this comment

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

I wonder if this might conflict with the changes in PR #26521. cc: @tdcmeehan

Copy link
Copy Markdown
Contributor Author

@zation99 zation99 Nov 13, 2025

Choose a reason for hiding this comment

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

I had offline discussion with @tdcmeehan . There is no conflict given how legacy MV is handled separately.

analysis.unregisterMaterializedViewForAnalysis(table);
}
}

Expand Down
14 changes: 8 additions & 6 deletions ...o-memory/src/test/java/com/facebook/presto/plugin/memory/TestMemoryMaterializedViews.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,9 +552,10 @@ public void testMaterializedViewWithDataConsistencyDisabledAfterBaseTableDropped

assertUpdate("DROP TABLE drop_consistency_test");

assertQuery(session, "SELECT COUNT(*) FROM mv_drop_consistency", "SELECT 2");
assertQuery(session, "SELECT * FROM mv_drop_consistency ORDER BY id",
"VALUES (1, 'initial'), (2, 'data')");
assertQueryFails(session, "SELECT COUNT(*) FROM mv_drop_consistency",
".*Table .* does not exist.*");
assertQueryFails(session, "SELECT * FROM mv_drop_consistency ORDER BY id",
".*Table .* does not exist.*");

assertQueryFails("REFRESH MATERIALIZED VIEW mv_drop_consistency",
".*Table .* does not exist.*");
Expand Down Expand Up @@ -582,9 +583,10 @@ public void testMaterializedViewWithDataConsistencyDisabledAfterBaseTableRenamed

assertUpdate("ALTER TABLE rename_consistency_test RENAME TO rename_consistency_test_new");

assertQuery(session, "SELECT COUNT(*) FROM mv_rename_consistency", "SELECT 2");
assertQuery(session, "SELECT * FROM mv_rename_consistency ORDER BY id",
"VALUES (1, 'initial'), (2, 'data')");
assertQueryFails(session, "SELECT COUNT(*) FROM mv_rename_consistency",
".*Table .* does not exist.*");
assertQueryFails(session, "SELECT * FROM mv_rename_consistency ORDER BY id",
".*Table .* does not exist.*");

assertQueryFails("REFRESH MATERIALIZED VIEW mv_rename_consistency",
".*Table .* does not exist.*");
Expand Down
Loading