xiangfu0 commented on code in PR #18681:
URL: https://github.com/apache/pinot/pull/18681#discussion_r3719077671
##########
pinot-materialized-view/src/test/java/org/apache/pinot/materializedview/rewrite/AggregationSubsumptionStrategyTest.java:
##########
@@ -894,4 +894,150 @@ public void
testFinerMaterializedViewSketchWithOrderByAlias() {
.getFunctionCall().getOperands().get(0).getIdentifier().getName(),
"raw_hll_FlightNum");
assertTrue(rewritten.getOrderByList().get(0).toString().contains("raw_hll_FlightNum"));
}
+
+ /// =======================================================================
+ /// Scalar grouping function support (e.g. DATETRUNC)
+ /// =======================================================================
+
+ /// A scalar grouping function (DATETRUNC) in the SELECT list must be
treated as a plain
+ /// projection — a direct MV column hit is sufficient. It must NOT be routed
through the
+ /// aggregation-equivalence path (which would reject it for lack of a
re-aggregation rule).
+ @Test
+ public void testScalarGroupingFunctionExactMatch() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders GROUP BY
DATETRUNC('DAY', ts)");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result, "Scalar grouping function should match via direct MV
projection");
+ assertEquals(result.getCost(), 6.0);
Review Comment:
Addressed in `d982480aa2d1a012e8f53b411f79e7eebd3c8797`. Removed the exact
`6.0` cost assertion from `testScalarGroupingFunctionExactMatch`; the test
still verifies the rewrite and structured GROUP BY/SELECT AST, including the
`day` and `sum_rev` MV columns. `AggregationSubsumptionStrategyTest`: 48/48
passed on JDK 25.
##########
pinot-materialized-view/src/test/java/org/apache/pinot/materializedview/rewrite/AggregationSubsumptionStrategyTest.java:
##########
@@ -894,4 +894,150 @@ public void
testFinerMaterializedViewSketchWithOrderByAlias() {
.getFunctionCall().getOperands().get(0).getIdentifier().getName(),
"raw_hll_FlightNum");
assertTrue(rewritten.getOrderByList().get(0).toString().contains("raw_hll_FlightNum"));
}
+
+ /// =======================================================================
+ /// Scalar grouping function support (e.g. DATETRUNC)
+ /// =======================================================================
+
+ /// A scalar grouping function (DATETRUNC) in the SELECT list must be
treated as a plain
+ /// projection — a direct MV column hit is sufficient. It must NOT be routed
through the
+ /// aggregation-equivalence path (which would reject it for lack of a
re-aggregation rule).
+ @Test
+ public void testScalarGroupingFunctionExactMatch() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders GROUP BY
DATETRUNC('DAY', ts)");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result, "Scalar grouping function should match via direct MV
projection");
+ assertEquals(result.getCost(), 6.0);
+
+ PinotQuery rewritten = result.getMaterializedViewQuery();
+
+ /// GROUP BY remapped to the MV column.
+ assertNotNull(rewritten.getGroupByList());
+ assertEquals(rewritten.getGroupByList().size(), 1);
+ assertEquals(rewritten.getGroupByList().get(0).getIdentifier().getName(),
"day");
+
+ List<Expression> selectList = rewritten.getSelectList();
+ assertEquals(selectList.size(), 2);
+
+ /// DATETRUNC('DAY', ts) → day AS datetrunc('DAY', ts) (direct projection,
alias preserves name).
+ Function dayAlias = selectList.get(0).getFunctionCall();
+ assertNotNull(dayAlias);
+ assertEquals(dayAlias.getOperator(), "as");
+ assertEquals(dayAlias.getOperands().get(0).getIdentifier().getName(),
"day");
+
+ /// SUM(revenue) → SUM(sum_rev) AS sum(revenue).
+ Function sumAlias = selectList.get(1).getFunctionCall();
+ assertNotNull(sumAlias);
+ assertEquals(sumAlias.getOperator(), "as");
+
assertEquals(sumAlias.getOperands().get(0).getFunctionCall().getOperator(),
"sum");
+
assertEquals(sumAlias.getOperands().get(0).getFunctionCall().getOperands().get(0).getIdentifier().getName(),
+ "sum_rev");
+ }
+
+ /// A scalar grouping function used in ORDER BY must remap to the MV column.
+ @Test
+ public void testScalarGroupingFunctionInOrderBy() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders "
+ + "GROUP BY DATETRUNC('DAY', ts) ORDER BY DATETRUNC('DAY', ts)
DESC");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result);
+ PinotQuery rewritten = result.getMaterializedViewQuery();
+ assertNotNull(rewritten.getOrderByList());
+ assertEquals(rewritten.getOrderByList().size(), 1);
+ assertTrue(rewritten.getOrderByList().get(0).toString().contains("day"),
+ "ORDER BY scalar grouping function should remap to the MV column");
+ }
+
+ /// A scalar grouping function used in HAVING must remap to the MV column.
+ @Test
+ public void testScalarGroupingFunctionInHaving() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders "
+ + "GROUP BY DATETRUNC('DAY', ts) HAVING DATETRUNC('DAY', ts) > 0");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result);
+ PinotQuery rewritten = result.getMaterializedViewQuery();
+ assertNotNull(rewritten.getHavingExpression());
+ assertTrue(rewritten.getHavingExpression().toString().contains("day"),
+ "HAVING scalar grouping function should remap to the MV column");
+ }
+
+ /// A scalar function not present in the MV projection (different truncation
unit) must reject.
+ @Test
+ public void testNoMatchScalarFunctionNotMaterialized() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('MONTH', ts), SUM(revenue) FROM orders GROUP BY
DATETRUNC('MONTH', ts)");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNull(result, "DATETRUNC('MONTH', ts) is not a materialized
projection and must be rejected");
+ }
+
+ /// MV is grouped by a superset of the user keys (DATETRUNC('DAY', ts),
city) while the user
+ /// groups only by DATETRUNC('DAY', ts). Re-aggregation is non-trivial here
(multiple MV rows
+ /// collapse per day), and the scalar grouping function must still resolve
as a direct MV hit.
+ @Test
+ public void testScalarGroupingFunctionFinerMaterializedViewGranularity() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, city, SUM(revenue) AS sum_rev
FROM orders "
+ + "GROUP BY DATETRUNC('DAY', ts), city";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders GROUP BY
DATETRUNC('DAY', ts)");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result, "Finer MV granularity with a scalar grouping key
should re-aggregate");
+ assertEquals(result.getCost(), 6.0);
Review Comment:
Addressed in `d982480aa2d1a012e8f53b411f79e7eebd3c8797`. Removed the exact
`6.0` cost assertion from
`testScalarGroupingFunctionFinerMaterializedViewGranularity`; the test retains
stable assertions for re-aggregation, GROUP BY remapping, aliases, and the
rewritten aggregate operand. `AggregationSubsumptionStrategyTest`: 48/48 passed
on JDK 25.
##########
pinot-materialized-view/src/test/java/org/apache/pinot/materializedview/rewrite/AggregationSubsumptionStrategyTest.java:
##########
@@ -894,4 +894,150 @@ public void
testFinerMaterializedViewSketchWithOrderByAlias() {
.getFunctionCall().getOperands().get(0).getIdentifier().getName(),
"raw_hll_FlightNum");
assertTrue(rewritten.getOrderByList().get(0).toString().contains("raw_hll_FlightNum"));
}
+
+ /// =======================================================================
+ /// Scalar grouping function support (e.g. DATETRUNC)
+ /// =======================================================================
+
+ /// A scalar grouping function (DATETRUNC) in the SELECT list must be
treated as a plain
+ /// projection — a direct MV column hit is sufficient. It must NOT be routed
through the
+ /// aggregation-equivalence path (which would reject it for lack of a
re-aggregation rule).
+ @Test
+ public void testScalarGroupingFunctionExactMatch() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders GROUP BY
DATETRUNC('DAY', ts)");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result, "Scalar grouping function should match via direct MV
projection");
+ assertEquals(result.getCost(), 6.0);
Review Comment:
Addressed in `d982480aa2d1a012e8f53b411f79e7eebd3c8797`. Removed the exact
`6.0` cost assertion from `testScalarGroupingFunctionExactMatch`; the test
still verifies the rewrite and structured GROUP BY/SELECT AST, including the
`day` and `sum_rev` MV columns. `AggregationSubsumptionStrategyTest`: 48/48
passed on JDK 25.
##########
pinot-materialized-view/src/main/java/org/apache/pinot/materializedview/rewrite/strategy/AggregationSubsumptionStrategy.java:
##########
@@ -347,8 +347,8 @@ private List<Expression>
buildReAggSelectList(List<Expression> userSelectList,
String userAlias = MaterializedViewMatchUtils.extractUserAlias(expr);
Expression rewritten;
- if (viewProjectionMap.containsKey(stripped)
- && stripped.getFunctionCall() == null) {
+ if (!CalciteSqlParser.isAggregateExpression(stripped) &&
viewProjectionMap.containsKey(stripped)) {
+ /// Plain column OR scalar grouping function: project the MV column
directly.
rewritten =
RequestUtils.getIdentifierExpression(viewProjectionMap.get(stripped));
} else {
rewritten = rewriteAggregationExpression(stripped, viewProjectionMap);
Review Comment:
Addressed in `d982480aa2d1a012e8f53b411f79e7eebd3c8797`.
`buildReAggSelectList` now has the explicit three-way structure requested:
direct MV projection for non-aggregates with a hit, aggregation rewrite for
aggregate expressions, and a clear `IllegalStateException` for an unprojected
non-aggregate. Unit tests passed 48/48 and the materialized-view cluster
integration test passed 12/12.
##########
pinot-materialized-view/src/test/java/org/apache/pinot/materializedview/rewrite/AggregationSubsumptionStrategyTest.java:
##########
@@ -894,4 +894,150 @@ public void
testFinerMaterializedViewSketchWithOrderByAlias() {
.getFunctionCall().getOperands().get(0).getIdentifier().getName(),
"raw_hll_FlightNum");
assertTrue(rewritten.getOrderByList().get(0).toString().contains("raw_hll_FlightNum"));
}
+
+ /// =======================================================================
+ /// Scalar grouping function support (e.g. DATETRUNC)
+ /// =======================================================================
+
+ /// A scalar grouping function (DATETRUNC) in the SELECT list must be
treated as a plain
+ /// projection — a direct MV column hit is sufficient. It must NOT be routed
through the
+ /// aggregation-equivalence path (which would reject it for lack of a
re-aggregation rule).
+ @Test
+ public void testScalarGroupingFunctionExactMatch() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders GROUP BY
DATETRUNC('DAY', ts)");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result, "Scalar grouping function should match via direct MV
projection");
+ assertEquals(result.getCost(), 6.0);
+
+ PinotQuery rewritten = result.getMaterializedViewQuery();
+
+ /// GROUP BY remapped to the MV column.
+ assertNotNull(rewritten.getGroupByList());
+ assertEquals(rewritten.getGroupByList().size(), 1);
+ assertEquals(rewritten.getGroupByList().get(0).getIdentifier().getName(),
"day");
+
+ List<Expression> selectList = rewritten.getSelectList();
+ assertEquals(selectList.size(), 2);
+
+ /// DATETRUNC('DAY', ts) → day AS datetrunc('DAY', ts) (direct projection,
alias preserves name).
+ Function dayAlias = selectList.get(0).getFunctionCall();
+ assertNotNull(dayAlias);
+ assertEquals(dayAlias.getOperator(), "as");
+ assertEquals(dayAlias.getOperands().get(0).getIdentifier().getName(),
"day");
+
+ /// SUM(revenue) → SUM(sum_rev) AS sum(revenue).
+ Function sumAlias = selectList.get(1).getFunctionCall();
+ assertNotNull(sumAlias);
+ assertEquals(sumAlias.getOperator(), "as");
+
assertEquals(sumAlias.getOperands().get(0).getFunctionCall().getOperator(),
"sum");
+
assertEquals(sumAlias.getOperands().get(0).getFunctionCall().getOperands().get(0).getIdentifier().getName(),
+ "sum_rev");
+ }
+
+ /// A scalar grouping function used in ORDER BY must remap to the MV column.
+ @Test
+ public void testScalarGroupingFunctionInOrderBy() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders "
+ + "GROUP BY DATETRUNC('DAY', ts) ORDER BY DATETRUNC('DAY', ts)
DESC");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result);
+ PinotQuery rewritten = result.getMaterializedViewQuery();
+ assertNotNull(rewritten.getOrderByList());
+ assertEquals(rewritten.getOrderByList().size(), 1);
+ assertTrue(rewritten.getOrderByList().get(0).toString().contains("day"),
+ "ORDER BY scalar grouping function should remap to the MV column");
+ }
+
+ /// A scalar grouping function used in HAVING must remap to the MV column.
+ @Test
+ public void testScalarGroupingFunctionInHaving() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders "
+ + "GROUP BY DATETRUNC('DAY', ts) HAVING DATETRUNC('DAY', ts) > 0");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result);
+ PinotQuery rewritten = result.getMaterializedViewQuery();
+ assertNotNull(rewritten.getHavingExpression());
+ assertTrue(rewritten.getHavingExpression().toString().contains("day"),
+ "HAVING scalar grouping function should remap to the MV column");
+ }
+
+ /// A scalar function not present in the MV projection (different truncation
unit) must reject.
+ @Test
+ public void testNoMatchScalarFunctionNotMaterialized() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, SUM(revenue) AS sum_rev FROM
orders "
+ + "GROUP BY DATETRUNC('DAY', ts)";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('MONTH', ts), SUM(revenue) FROM orders GROUP BY
DATETRUNC('MONTH', ts)");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNull(result, "DATETRUNC('MONTH', ts) is not a materialized
projection and must be rejected");
+ }
+
+ /// MV is grouped by a superset of the user keys (DATETRUNC('DAY', ts),
city) while the user
+ /// groups only by DATETRUNC('DAY', ts). Re-aggregation is non-trivial here
(multiple MV rows
+ /// collapse per day), and the scalar grouping function must still resolve
as a direct MV hit.
+ @Test
+ public void testScalarGroupingFunctionFinerMaterializedViewGranularity() {
+ String definedSql =
+ "SELECT DATETRUNC('DAY', ts) AS day, city, SUM(revenue) AS sum_rev
FROM orders "
+ + "GROUP BY DATETRUNC('DAY', ts), city";
+ MaterializedViewCacheEntry entry = createEntry("mv_orders_OFFLINE",
"orders", definedSql);
+
+ PinotQuery userQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT DATETRUNC('DAY', ts), SUM(revenue) FROM orders GROUP BY
DATETRUNC('DAY', ts)");
+ MaterializedViewRewritePlan result = _strategy.match(userQuery, entry);
+
+ assertNotNull(result, "Finer MV granularity with a scalar grouping key
should re-aggregate");
+ assertEquals(result.getCost(), 6.0);
Review Comment:
Addressed in `d982480aa2d1a012e8f53b411f79e7eebd3c8797`. Removed the exact
`6.0` cost assertion from
`testScalarGroupingFunctionFinerMaterializedViewGranularity`; the test retains
stable assertions for re-aggregation, GROUP BY remapping, aliases, and the
rewritten aggregate operand. `AggregationSubsumptionStrategyTest`: 48/48 passed
on JDK 25.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
