This is an automated email from the ASF dual-hosted git repository.
gortiz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 6ac0a577da5 Reject QUALIFY in the single-stage engine instead of
silently ignoring it (#19553)
6ac0a577da5 is described below
commit 6ac0a577da57b45cdaecbaffe0ca256d5d5b04e1
Author: Gonzalo Ortiz Jaureguizar <[email protected]>
AuthorDate: Tue Sep 15 11:45:48 2026 +0200
Reject QUALIFY in the single-stage engine instead of silently ignoring it
(#19553)
---
.../apache/pinot/sql/parsers/CalciteSqlParser.java | 10 +++++
.../sql/parsers/parser/TableNameExtractor.java | 5 +++
.../pinot/sql/parsers/CalciteSqlParserTest.java | 37 ++++++++++++++++++
.../sql/parsers/parser/TableNameExtractorTest.java | 14 +++++++
.../resources/queries/WindowFunctionPlans.json | 15 ++++++++
.../test/resources/queries/WindowFunctions.json | 44 +++++++++++++++++++++-
6 files changed, 124 insertions(+), 1 deletion(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
index 5a060850385..d82c1a96922 100644
---
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
+++
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
@@ -625,6 +625,16 @@ public class CalciteSqlParser {
if (havingNode != null) {
pinotQuery.setHavingExpression(toExpression(havingNode));
}
+ // QUALIFY
+ // QUALIFY filters rows after the window functions of the SELECT list are
evaluated, the way HAVING filters them
+ // after aggregation. The single-stage engine has no window functions, and
PinotQuery has no field to carry the
+ // predicate, so it must be rejected rather than left unread: an unread
QUALIFY is a silently wrong answer.
+ if (selectNode.getQualify() != null) {
+ throw new SqlCompilationException("QUALIFY is not supported by the
single-stage query engine. Use the "
+ + "multi-stage query engine to filter on the result of a window
function. If the predicate does not "
+ + "reference a window function, rewrite it as a WHERE clause (to
filter on columns) or as a HAVING clause "
+ + "(to filter on aggregates of a GROUP BY query).");
+ }
// ORDER-BY
SqlNodeList orderByNodeList = selectNode.getOrderList();
if (orderByNodeList != null) {
diff --git
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/parser/TableNameExtractor.java
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/parser/TableNameExtractor.java
index 9b1b424f11c..edce1054533 100644
---
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/parser/TableNameExtractor.java
+++
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/parser/TableNameExtractor.java
@@ -167,6 +167,11 @@ public class TableNameExtractor {
if (select.getHaving() != null) {
extractTableNames(select.getHaving());
}
+ // QUALIFY is rejected by the single-stage engine but legal for the
multi-stage one, which is what this extractor
+ // runs on, so a subquery hiding in it still has to be walked.
+ if (select.getQualify() != null) {
+ extractTableNames(select.getQualify());
+ }
if (select.getOrderList() != null) {
visitNodeList(select.getOrderList());
}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlParserTest.java
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlParserTest.java
index 47d64c17a65..dec47000efb 100644
---
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlParserTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlParserTest.java
@@ -30,6 +30,8 @@ import org.testng.annotations.Test;
import static
org.apache.pinot.sql.parsers.CalciteSqlParser.CALCITE_SQL_PARSER_IDENTIFIER_MAX_LENGTH;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertThrows;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
/// Tests for CalciteSqlParser.
@@ -255,4 +257,39 @@ public class CalciteSqlParserTest {
Function function = filterExpr.getFunctionCall();
assertEquals(function.getOperands().get(1).getLiteral().getStringValue(),
"It's fine");
}
+
+ /// QUALIFY used to be parsed and then silently dropped, so a query relying
on it returned every row instead of the
+ /// filtered ones. The single-stage engine cannot evaluate it, so it must
fail loudly.
+ @Test
+ public void testQualifyIsRejected() {
+ // The idiom from the bug report: keep the latest row per partition.
+ assertQualifyRejected("SELECT city, category FROM myTable "
+ + "QUALIFY ROW_NUMBER() OVER (PARTITION BY city ORDER BY orderDate
DESC) = 1 ORDER BY 1, 2 LIMIT 100");
+ // The same idiom written against a SELECT-list alias, which is the more
common spelling. It reaches the check
+ // only because toExpression() compiles the SqlWindow operand to a literal
instead of throwing; assert on the
+ // QUALIFY message so tightening that branch cannot silently downgrade
this to "Unsupported sql node".
+ assertQualifyRejected("SELECT city, ROW_NUMBER() OVER (PARTITION BY city
ORDER BY orderDate DESC) AS rn "
+ + "FROM myTable QUALIFY rn = 1");
+ // Without a window function QUALIFY is still rejected: the single-stage
engine only applies HAVING to GROUP BY
+ // queries, so folding the predicate into HAVING would drop it just as
silently for the other query shapes.
+ // These shapes are not valid SQL either -- the multi-stage engine rejects
them with "QUALIFY expression must
+ // contain a window function" -- which is why the message conditions its
two remedies on whether the predicate
+ // references a window function rather than recommending either one
outright.
+ assertQualifyRejected("SELECT city FROM myTable QUALIFY city > 'a'");
+ assertQualifyRejected("SELECT city, COUNT(*) FROM myTable GROUP BY city
QUALIFY COUNT(*) > 5");
+ // A QUALIFY next to a HAVING must not be swallowed by the HAVING being
present.
+ assertQualifyRejected(
+ "SELECT city, COUNT(*) FROM myTable GROUP BY city HAVING COUNT(*) > 3
QUALIFY COUNT(*) > 5");
+ // Subqueries are compiled through the same path.
+ assertQualifyRejected("SELECT city FROM (SELECT city FROM myTable QUALIFY
city > 'a') AS t");
+ // EXPLAIN unwraps to the same SELECT node.
+ assertQualifyRejected("EXPLAIN PLAN FOR SELECT city FROM myTable QUALIFY
city > 'a'");
+ }
+
+ private void assertQualifyRejected(String query) {
+ SqlCompilationException e =
+ expectThrows(SqlCompilationException.class, () ->
CalciteSqlParser.compileToPinotQuery(query));
+ assertTrue(e.getMessage().contains("QUALIFY is not supported by the
single-stage query engine"),
+ "Unexpected message for query '" + query + "': " + e.getMessage());
+ }
}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/parser/TableNameExtractorTest.java
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/parser/TableNameExtractorTest.java
index 97dbce96d2a..4eaa3f8ab93 100644
---
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/parser/TableNameExtractorTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/parser/TableNameExtractorTest.java
@@ -269,6 +269,20 @@ public class TableNameExtractorTest {
assertTrue(Arrays.asList(tableNames).contains("orders"), "Should contain
orders table");
}
+ @Test
+ public void testResolveTableNameWithQualifySubquery() {
+ // QUALIFY is legal for the multi-stage engine, which is what this
extractor runs on, so a table referenced only
+ // from inside it must still be found.
+ String qualifySubqueryQuery = "SELECT u.id, ROW_NUMBER() OVER (PARTITION
BY u.id) AS rn FROM users u "
+ + "QUALIFY rn <= (SELECT COUNT(*) FROM orders o WHERE o.user_id =
u.id)";
+ String[] tableNames =
TableNameExtractor.resolveTableName(qualifySubqueryQuery);
+
+ assertNotNull(tableNames, "Table names should not be null");
+ assertEquals(tableNames.length, 2, "Should resolve two tables");
+ assertTrue(Arrays.asList(tableNames).contains("users"), "Should contain
users table");
+ assertTrue(Arrays.asList(tableNames).contains("orders"), "Should contain
orders table from QUALIFY subquery");
+ }
+
@Test(expectedExceptions = RuntimeException.class)
public void testResolveTableNameWithInvalidQuery() {
String[] tableNames = TableNameExtractor.resolveTableName("INVALID SQL
QUERY");
diff --git
a/pinot-query-planner/src/test/resources/queries/WindowFunctionPlans.json
b/pinot-query-planner/src/test/resources/queries/WindowFunctionPlans.json
index cea44cc7f06..1fc70530dac 100644
--- a/pinot-query-planner/src/test/resources/queries/WindowFunctionPlans.json
+++ b/pinot-query-planner/src/test/resources/queries/WindowFunctionPlans.json
@@ -582,6 +582,21 @@
"\n"
]
},
+ {
+ "description": "QUALIFY desugars to a filter over the window, keeping
the top row per partition",
+ "notes": "QUALIFY is rejected by the single-stage engine (it has no
window functions), so this is the only engine that runs it",
+ "sql": "EXPLAIN PLAN FOR SELECT a.col1, a.col2 FROM a QUALIFY
ROW_NUMBER() OVER(PARTITION BY a.col2 ORDER BY a.col3 DESC) = 1",
+ "output": [
+ "Execution Plan",
+ "\nLogicalProject(col1=[$0], col2=[$1])",
+ "\n LogicalFilter(condition=[=($3, 1)])",
+ "\n LogicalWindow(window#0=[window(partition {1} order by [2
DESC] rows between UNBOUNDED PRECEDING and CURRENT ROW aggs [ROW_NUMBER()])])",
+ "\n PinotLogicalSortExchange(distribution=[hash[1]],
collation=[[2 DESC]], isSortOnSender=[false], isSortOnReceiver=[true])",
+ "\n LogicalProject(col1=[$0], col2=[$1], col3=[$2])",
+ "\n PinotLogicalTableScan(table=[[default, a]])",
+ "\n"
+ ]
+ },
{
"description": "single OVER(PARTITION BY) only with alias",
"sql": "EXPLAIN PLAN FOR SELECT SUM(a.col3) OVER(PARTITION BY a.col2)
AS sum FROM a",
diff --git
a/pinot-query-runtime/src/test/resources/queries/WindowFunctions.json
b/pinot-query-runtime/src/test/resources/queries/WindowFunctions.json
index 44015a693d8..868d44d9cc3 100644
--- a/pinot-query-runtime/src/test/resources/queries/WindowFunctions.json
+++ b/pinot-query-runtime/src/test/resources/queries/WindowFunctions.json
@@ -5853,4 +5853,46 @@
}
]
}
-}
\ No newline at end of file
+ ,
+ "qualify_window_function_filter": {
+ "tables": {
+ "tbl": {
+ "schema": [
+ {"name": "city", "type": "STRING"},
+ {"name": "category", "type": "STRING"},
+ {"name": "order_date", "type": "INT"}
+ ],
+ "inputs": [
+ ["Athens", "Office Supplies", 3],
+ ["Athens", "Technology", 1],
+ ["Madrid", "Technology", 5],
+ ["Madrid", "Office Supplies", 2],
+ ["Springfield", "Furniture", 7]
+ ]
+ }
+ },
+ "queries": [
+ {
+ "description": "QUALIFY keeps only the latest row per city",
+ "notes": "The single-stage engine rejects QUALIFY (it has no window
functions), so this is the only engine that runs it",
+ "sql": "SELECT city, category FROM {tbl} QUALIFY ROW_NUMBER()
OVER(PARTITION BY city ORDER BY order_date DESC) = 1 ORDER BY city",
+ "keepOutputRowOrder": true,
+ "outputs": [
+ ["Athens", "Office Supplies"],
+ ["Madrid", "Technology"],
+ ["Springfield", "Furniture"]
+ ]
+ },
+ {
+ "description": "QUALIFY on a SELECT-list window alias",
+ "sql": "SELECT city, ROW_NUMBER() OVER(PARTITION BY city ORDER BY
order_date DESC) AS rn FROM {tbl} QUALIFY rn = 1 ORDER BY city",
+ "keepOutputRowOrder": true,
+ "outputs": [
+ ["Athens", 1],
+ ["Madrid", 1],
+ ["Springfield", 1]
+ ]
+ }
+ ]
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
