Skip to content

Commit 8b16dac

Browse files
ceekay47facebook-github-bot
authored andcommitted
feat(planner): Support GROUP BY and ORDER BY ordinals in MV query rewriting (prestodb#27422) (prestodb#27422)
Summary: Queries using GROUP BY/ORDER BY ordinals (e.g. `GROUP BY 1`) silently fell back to the base table because the MV optimizer runs before the analyzer resolves ordinals to column references. Fix by resolving ordinals to SELECT expressions during MV validation and passing them through unchanged during rewriting. ``` == RELEASE NOTES == General Changes * Add support for ``GROUP BY`` and ``ORDER BY`` ordinal references in materialized view query rewriting. Previously, queries like ``SELECT a, SUM(b) FROM t GROUP BY 1`` would silently skip materialized view optimization. ``` Differential Revision: D97920227 Pulled By: ceekay47
1 parent b051f49 commit 8b16dac

File tree

2 files changed

+72
-6
lines changed

2 files changed

+72
-6
lines changed

presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/MaterializedViewQueryOptimizer.java

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import com.facebook.presto.sql.tree.Join;
5757
import com.facebook.presto.sql.tree.Lateral;
5858
import com.facebook.presto.sql.tree.LogicalBinaryExpression;
59+
import com.facebook.presto.sql.tree.LongLiteral;
5960
import com.facebook.presto.sql.tree.Node;
6061
import com.facebook.presto.sql.tree.OrderBy;
6162
import com.facebook.presto.sql.tree.QualifiedName;
@@ -109,6 +110,7 @@
109110
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.NOT_SUPPORTED;
110111
import static com.facebook.presto.sql.relational.Expressions.call;
111112
import static com.facebook.presto.util.AnalyzerUtil.createParsingOptions;
113+
import static java.lang.Math.toIntExact;
112114
import static java.lang.String.format;
113115
import static java.util.Objects.requireNonNull;
114116

@@ -495,18 +497,38 @@ protected Node visitQuerySpecification(QuerySpecification node, Void context)
495497
removablePrefix = Optional.of(new Identifier(baseTable.getName().toString()));
496498
}
497499
if (node.getGroupBy().isPresent()) {
500+
List<SelectItem> selectItems = node.getSelect().getSelectItems();
498501
ImmutableSet.Builder<Expression> expressionsInGroupByBuilder = ImmutableSet.builder();
499502
for (GroupingElement element : node.getGroupBy().get().getGroupingElements()) {
500503
element = removeGroupingElementPrefix(element, removablePrefix);
501504
Optional<Set<Expression>> groupByOfMaterializedView = materializedViewInfo.getGroupBy();
502505
if (groupByOfMaterializedView.isPresent()) {
503506
for (Expression expression : element.getExpressions()) {
504-
if (!groupByOfMaterializedView.get().contains(expression) || !materializedViewInfo.getBaseToViewColumnMap().containsKey(expression)) {
507+
// Resolve ordinal references (e.g. GROUP BY 1) to the corresponding SELECT expression
508+
Expression resolved = expression;
509+
if (expression instanceof LongLiteral) {
510+
int ordinal = toIntExact(((LongLiteral) expression).getValue());
511+
if (ordinal < 1 || ordinal > selectItems.size()) {
512+
throw new SemanticException(NOT_SUPPORTED, expression, "GROUP BY ordinal %d is out of range (1 to %d)", ordinal, selectItems.size());
513+
}
514+
SelectItem selectItem = selectItems.get(ordinal - 1);
515+
if (selectItem instanceof SingleColumn) {
516+
resolved = removeExpressionPrefix(((SingleColumn) selectItem).getExpression(), removablePrefix);
517+
}
518+
else {
519+
throw new IllegalStateException("GROUP BY ordinal references non-single-column select item");
520+
}
521+
}
522+
if (!groupByOfMaterializedView.get().contains(resolved) || !materializedViewInfo.getBaseToViewColumnMap().containsKey(resolved)) {
505523
throw new IllegalStateException(format("Grouping element %s is not present in materialized view groupBy field", element));
506524
}
525+
// Store the resolved expression so visitSingleColumn can match against it
526+
expressionsInGroupByBuilder.add(resolved);
507527
}
508528
}
509-
expressionsInGroupByBuilder.addAll(element.getExpressions());
529+
else {
530+
expressionsInGroupByBuilder.addAll(element.getExpressions());
531+
}
510532
}
511533
expressionsInGroupBy = Optional.of(expressionsInGroupByBuilder.build());
512534
}
@@ -715,8 +737,11 @@ protected Node visitOrderBy(OrderBy node, Void context)
715737
ImmutableList.Builder<SortItem> rewrittenOrderBy = ImmutableList.builder();
716738
for (SortItem sortItem : node.getSortItems()) {
717739
sortItem = removeSortItemPrefix(sortItem, removablePrefix);
718-
if (!materializedViewInfo.getBaseToViewColumnMap().containsKey(sortItem.getSortKey())) {
719-
throw new IllegalStateException(format("Sort key %s is not present in materialized view select fields", sortItem.getSortKey()));
740+
// Ordinal references (e.g. ORDER BY 3) refer to SELECT items which are already validated
741+
Expression sortKey = sortItem.getSortKey();
742+
if (!(sortKey instanceof LongLiteral)
743+
&& !materializedViewInfo.getBaseToViewColumnMap().containsKey(sortKey)) {
744+
throw new IllegalStateException(format("Sort key %s is not present in materialized view select fields", sortKey));
720745
}
721746
rewrittenOrderBy.add((SortItem) process(sortItem, context));
722747
}
@@ -726,15 +751,26 @@ protected Node visitOrderBy(OrderBy node, Void context)
726751
@Override
727752
protected Node visitSortItem(SortItem node, Void context)
728753
{
729-
return new SortItem((Expression) process(node.getSortKey(), context), node.getOrdering(), node.getNullOrdering());
754+
Expression sortKey = node.getSortKey();
755+
// Ordinal references (e.g. ORDER BY 1) refer to SELECT positions which are already rewritten; pass through unchanged
756+
if (sortKey instanceof LongLiteral) {
757+
return node;
758+
}
759+
return new SortItem((Expression) process(sortKey, context), node.getOrdering(), node.getNullOrdering());
730760
}
731761

732762
@Override
733763
protected Node visitSimpleGroupBy(SimpleGroupBy node, Void context)
734764
{
735765
ImmutableList.Builder<Expression> rewrittenSimpleGroupBy = ImmutableList.builder();
736766
for (Expression column : node.getExpressions()) {
737-
rewrittenSimpleGroupBy.add((Expression) process(removeExpressionPrefix(column, removablePrefix), context));
767+
// Ordinal references (e.g. GROUP BY 1) refer to SELECT positions which are already rewritten; pass through unchanged
768+
if (column instanceof LongLiteral) {
769+
rewrittenSimpleGroupBy.add(column);
770+
}
771+
else {
772+
rewrittenSimpleGroupBy.add((Expression) process(removeExpressionPrefix(column, removablePrefix), context));
773+
}
738774
}
739775
return new SimpleGroupBy(rewrittenSimpleGroupBy.build());
740776
}

presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestMaterializedViewQueryOptimizer.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,36 @@ public void testWithOrderBy()
255255
assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);
256256
}
257257

258+
@Test
259+
public void testWithGroupByOrdinals()
260+
{
261+
String originalViewSql = format("SELECT a as mv_a, b, c as mv_c FROM %s", BASE_TABLE_1);
262+
String baseQuerySql = format("SELECT SUM(a * b), MAX(a + b), c FROM %s GROUP BY 3", BASE_TABLE_1);
263+
String expectedRewrittenSql = format("SELECT SUM(mv_a * b), MAX(mv_a + b), mv_c as c FROM %s GROUP BY 3", VIEW_1);
264+
265+
assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);
266+
}
267+
268+
@Test
269+
public void testWithOrderByOrdinals()
270+
{
271+
String originalViewSql = format("SELECT a as mv_a, b, c as mv_c FROM %s", BASE_TABLE_1);
272+
String baseQuerySql = format("SELECT a, b, c FROM %s ORDER BY 3 ASC, 2 DESC, 1", BASE_TABLE_1);
273+
String expectedRewrittenSql = format("SELECT mv_a as a, b, mv_c as c FROM %s ORDER BY 3 ASC, 2 DESC, 1", VIEW_1);
274+
275+
assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);
276+
}
277+
278+
@Test
279+
public void testWithGroupByAndOrderByOrdinals()
280+
{
281+
String originalViewSql = format("SELECT MAX(a) as mv_max_a, b FROM %s GROUP BY b", BASE_TABLE_1);
282+
String baseQuerySql = format("SELECT MAX(a), b FROM %s GROUP BY 2 ORDER BY 1 DESC, 2 ASC", BASE_TABLE_1);
283+
String expectedRewrittenSql = format("SELECT MAX(mv_max_a), b FROM %s GROUP BY 2 ORDER BY 1 DESC, 2 ASC", VIEW_1);
284+
285+
assertOptimizedQuery(baseQuerySql, expectedRewrittenSql, originalViewSql, BASE_TABLE_1, VIEW_1);
286+
}
287+
258288
@Test
259289
public void testWithNoMatchingBaseTable()
260290
{

0 commit comments

Comments
 (0)