Skip to content

Resolve table names in MV query optimizer for consistent matching #27059

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

Open
ceekay47 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Resolve table names in MV query optimizer for consistent matching #27059

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
13 changes: 13 additions & 0 deletions presto-main-base/src/main/java/com/facebook/presto/sql/MaterializedViewUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.facebook.presto.sql;

import com.facebook.presto.Session;
import com.facebook.presto.common.QualifiedObjectName;
import com.facebook.presto.common.predicate.NullableValue;
import com.facebook.presto.common.predicate.TupleDomain;
import com.facebook.presto.metadata.Metadata;
Expand All @@ -40,7 +41,9 @@
import com.facebook.presto.sql.tree.IsNullPredicate;
import com.facebook.presto.sql.tree.LogicalBinaryExpression;
import com.facebook.presto.sql.tree.QualifiedName;
import com.facebook.presto.sql.tree.Relation;
import com.facebook.presto.sql.tree.SymbolReference;
import com.facebook.presto.sql.tree.Table;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand All @@ -60,6 +63,7 @@
import static com.facebook.presto.common.predicate.TupleDomain.extractFixedValues;
import static com.facebook.presto.common.type.StandardTypes.HYPER_LOG_LOG;
import static com.facebook.presto.common.type.StandardTypes.VARBINARY;
import static com.facebook.presto.metadata.MetadataUtil.createQualifiedObjectName;
import static com.facebook.presto.sql.ExpressionUtils.combineDisjuncts;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.NOT_SUPPORTED;
import static com.facebook.presto.sql.tree.ArithmeticBinaryExpression.Operator.DIVIDE;
Expand Down Expand Up @@ -399,6 +403,15 @@ public static Expression convertMaterializedDataPredicatesToExpression(
}
}

public static Relation resolveTableName(Relation relation, Session session, Metadata metadata)
{
if (!(relation instanceof Table)) {
return relation;
}
QualifiedObjectName qualifiedTableName = createQualifiedObjectName(session, relation, ((Table) relation).getName(), metadata);
return new Table(QualifiedName.of(qualifiedTableName.getSchemaName(), qualifiedTableName.getObjectName()));
}

private static Expression convertSymbolReferencesToIdentifiers(Expression expression)
{
return ExpressionTreeRewriter.rewriteWith(new ExpressionRewriter<Void>()
Expand Down
4 changes: 3 additions & 1 deletion ...n-base/src/main/java/com/facebook/presto/sql/analyzer/MaterializedViewQueryOptimizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
import static com.facebook.presto.sql.MaterializedViewUtils.COUNT;
import static com.facebook.presto.sql.MaterializedViewUtils.NON_ASSOCIATIVE_REWRITE_FUNCTIONS;
import static com.facebook.presto.sql.MaterializedViewUtils.SUM;
import static com.facebook.presto.sql.MaterializedViewUtils.resolveTableName;
import static com.facebook.presto.sql.analyzer.MaterializedViewInformationExtractor.MaterializedViewInfo;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_TABLE;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.NOT_SUPPORTED;
Expand Down Expand Up @@ -641,7 +642,8 @@ protected Node visitAliasedRelation(AliasedRelation node, Void context)
@Override
protected Node visitRelation(Relation node, Void context)
{
if (materializedViewInfo.getBaseTable().isPresent() && node.equals(materializedViewInfo.getBaseTable().get())) {
if (materializedViewInfo.getBaseTable().isPresent() && resolveTableName(node, session, metadata)
.equals(resolveTableName(materializedViewInfo.getBaseTable().get(), session, metadata))) {
return materializedView;
}
throw new IllegalStateException("Mismatching table or non-supporting relation format in base query");
Expand Down
36 changes: 36 additions & 0 deletions ...se/src/test/java/com/facebook/presto/sql/analyzer/TestMaterializedViewQueryOptimizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,42 @@ public void testWithTableAlias()
assertOptimizedQuery(baseQuerySqlWithTablePrefix, expectedRewrittenSql, originalViewSqlWithTablePrefix, BASE_TABLE_1, VIEW_1);
}

@Test
public void testWithSchemaQualifiedTableName()
Comment on lines +453 to +454
Copy link
Contributor

@sourcery-ai sourcery-ai bot Jan 30, 2026

Choose a reason for hiding this comment

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

suggestion (testing): Add negative tests for mismatching schemas/table names to prove we don’t rewrite when we shouldn’t

To make the optimizer behavior more robustly tested, please add one or two negative cases where the schema-qualified name resolves to a different table than the MV base table (e.g., SESSION_SCHEMA.other_table or other_schema.base_table). These should assert that the MV rewrite does not occur (or that the correct exception path is taken), ensuring the new resolveTableName logic does not over-match tables.

Suggested implementation:

    @Test
    public void testWithSchemaQualifiedTableName()
    {
        String schemaQualifiedTable = SESSION_SCHEMA + "." + BASE_TABLE_1;

        String originalViewSql = format("SELECT a, b FROM %s", BASE_TABLE_1);
        String baseQuerySql = format("SELECT a, b FROM %s", schemaQualifiedTable);
        String expectedRewrittenSql = format("SELECT a, b FROM %s", VIEW_1);

        assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);

        originalViewSql = format("SELECT a, b FROM %s", schemaQualifiedTable);
        baseQuerySql = format("SELECT a, b FROM %s", BASE_TABLE_1);
        expectedRewrittenSql = format("SELECT a, b FROM %s", VIEW_1);

        assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);
    }

    @Test
    public void testWithSchemaQualifiedTableNameMismatchingTable()
    {
        // MV is defined over BASE_TABLE_1; query uses a different table in the same schema
        String schemaQualifiedOtherTable = SESSION_SCHEMA + "." + BASE_TABLE_2;

        String originalViewSql = format("SELECT a, b FROM %s", BASE_TABLE_1);
        String baseQuerySql = format("SELECT a, b FROM %s", schemaQualifiedOtherTable);

        // Verify that a different table name in the same schema does NOT trigger a rewrite
        assertNotOptimizedQuery(baseQuerySql, originalViewSql, BASE_TABLE_1, VIEW_1);
    }

    @Test
    public void testWithSchemaQualifiedTableNameMismatchingSchema()
    {
        // MV is defined over SESSION_SCHEMA.BASE_TABLE_1; query uses the same table name in a different schema
        String otherSchemaQualifiedTable = "other_schema." + BASE_TABLE_1;

        String originalViewSql = format("SELECT a, b FROM %s", SESSION_SCHEMA + "." + BASE_TABLE_1);
        String baseQuerySql = format("SELECT a, b FROM %s", otherSchemaQualifiedTable);

        // Verify that a different schema for the same table name does NOT trigger a rewrite
        assertNotOptimizedQuery(baseQuerySql, originalViewSql, BASE_TABLE_1, VIEW_1);

To fully wire these tests into the existing test harness, you will also need to:

  1. Ensure there is a BASE_TABLE_2 constant defined in this test class (or adjust testWithSchemaQualifiedTableNameMismatchingTable to use whatever second base-table constant already exists).
  2. Implement or reuse an assertion helper like assertNotOptimizedQuery(String baseQuerySql, String originalViewSql, String baseTable, String viewName) that:
    • Runs the materialized view optimizer on baseQuerySql given the provided MV definition (originalViewSql, baseTable, viewName).
    • Asserts that either:
      a) The optimized query text is identical to baseQuerySql (no rewrite occurred), or
      b) The optimizer throws the expected exception for non-rewritable queries (if that is the intended behavior).
  3. If such a helper already exists under a different name/signature (e.g., assertNoRewrite, assertNotRewrittenQuery, etc.), replace assertNotOptimizedQuery(...) calls with the appropriate existing helper and adjust the arguments accordingly.

{
String schemaQualifiedTable = SESSION_SCHEMA + "." + BASE_TABLE_1;

String originalViewSql = format("SELECT a, b FROM %s", BASE_TABLE_1);
String baseQuerySql = format("SELECT a, b FROM %s", schemaQualifiedTable);
String expectedRewrittenSql = format("SELECT a, b FROM %s", VIEW_1);

assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);

originalViewSql = format("SELECT a, b FROM %s", schemaQualifiedTable);
baseQuerySql = format("SELECT a, b FROM %s", BASE_TABLE_1);
expectedRewrittenSql = format("SELECT a, b FROM %s", VIEW_1);

assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);

originalViewSql = format("SELECT a, b FROM %s", schemaQualifiedTable);
baseQuerySql = format("SELECT a, b FROM %s", schemaQualifiedTable);
expectedRewrittenSql = format("SELECT a, b FROM %s", VIEW_1);

assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);

originalViewSql = format("SELECT a, b, c FROM %s", BASE_TABLE_1);
baseQuerySql = format("SELECT a, b FROM %s WHERE c > 10", schemaQualifiedTable);
expectedRewrittenSql = format("SELECT a, b FROM %s WHERE c > 10", VIEW_1);

assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);

originalViewSql = format("SELECT SUM(a) as sum_a, b FROM %s GROUP BY b", BASE_TABLE_1);
baseQuerySql = format("SELECT SUM(a), b FROM %s GROUP BY b", schemaQualifiedTable);
expectedRewrittenSql = format("SELECT SUM(sum_a), b FROM %s GROUP BY b", VIEW_1);

assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);
}

@Test
public void testAggregationWithTableAlias()
{
Expand Down
Loading