This is an automated email from the ASF dual-hosted git repository.
yashmayya 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 72ebec4fa21 Make logical EXPLAIN fail wherever query execution would
fail (#19373)
72ebec4fa21 is described below
commit 72ebec4fa21582647ea5f7e73c66dbd5a408d565
Author: Yash Mayya <[email protected]>
AuthorDate: Thu Aug 27 12:05:05 2026 -0400
Make logical EXPLAIN fail wherever query execution would fail (#19373)
---
.../java/org/apache/pinot/query/QueryEnvironment.java | 15 ++++++++++++++-
.../org/apache/pinot/query/QueryCompilationTest.java | 17 +++++++++++------
.../query/queries/ResourceBasedQueryPlansTest.java | 2 --
.../src/test/resources/queries/JoinPlans.json | 5 +++++
.../src/test/resources/queries/ValidationErrorPlan.json | 5 +++++
5 files changed, 35 insertions(+), 9 deletions(-)
diff --git
a/pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
b/pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
index 26fd33b32a2..bfa24cac481 100644
---
a/pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
+++
b/pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java
@@ -955,6 +955,9 @@ public class QueryEnvironment {
/// Explain the query plan.
/// The original query must be an EXPLAIN query and way it will be
explained depends on the options of the EXPLAIN
/// query and the [QueryEnvironment.Config] used to create the
[QueryEnvironment] that compiled this query.
+ ///
+ /// Unless the query explicitly asks for a logical plan only (`EXPLAIN
PLAN WITHOUT IMPLEMENTATION`), this builds
+ /// the dispatchable subplan and therefore fails wherever [#planQuery]
would.
public QueryEnvironment.QueryPlannerResult explain(long requestId,
@Nullable AskingServerStageExplainer.OnServerExplainer
onServerExplainer) {
try {
@@ -971,9 +974,17 @@ public class QueryEnvironment {
SqlExplainLevel level =
explain.getDetailLevel() == null ?
SqlExplainLevel.DIGEST_ATTRIBUTES : explain.getDetailLevel();
Set<String> tableNames =
RelToPlanNodeConverter.getTableNamesFromRelRoot(_relRoot.rel);
- if (!explain.withImplementation() || onServerExplainer == null) {
+ if (!explain.withImplementation()) {
+ // The query explicitly asked to skip implementation planning, so
render the logical plan as-is.
return getQueryPlannerResult(_plannerContext, null,
PlannerUtils.explainPlan(_relRoot.rel, format, level),
tableNames);
+ } else if (onServerExplainer == null) {
+ // Build the dispatchable subplan even though only the logical
plan is rendered: some planning errors are
+ // raised while converting the plan or assigning workers (e.g. a
`tableOptions` partition hint that
+ // disagrees with the table's actual partitioning), and EXPLAIN
must fail wherever execution would.
+ DispatchableSubPlan dispatchableSubPlan =
toDispatchableSubPlan(_relRoot, _plannerContext);
+ return getQueryPlannerResult(_plannerContext, dispatchableSubPlan,
+ PlannerUtils.explainPlan(_relRoot.rel, format, level),
dispatchableSubPlan.getTableNames());
} else {
Map<String, String> options = _sqlNodeAndOptions.getOptions();
boolean explainPlanVerbose =
QueryOptionsUtils.isExplainPlanVerbose(options);
@@ -995,6 +1006,8 @@ public class QueryEnvironment {
PlannerUtils.explainPlan(explainedNode, format, level),
dispatchableSubPlan.getTableNames());
}
}
+ } catch (QueryException e) {
+ throw e;
} catch (Exception e) {
throw new RuntimeException("Error explain query plan for: " +
_textQuery, e);
}
diff --git
a/pinot-query-planner/src/test/java/org/apache/pinot/query/QueryCompilationTest.java
b/pinot-query-planner/src/test/java/org/apache/pinot/query/QueryCompilationTest.java
index 6376839d748..30f74e5d60c 100644
---
a/pinot-query-planner/src/test/java/org/apache/pinot/query/QueryCompilationTest.java
+++
b/pinot-query-planner/src/test/java/org/apache/pinot/query/QueryCompilationTest.java
@@ -568,9 +568,11 @@ public class QueryCompilationTest extends
QueryEnvironmentTestBase {
public void testJoinPushTransitivePredicateLookupJoin() {
// PinotJoinPushTransitivePredicatesRule
// should not push to the right under lookup join hint
+ // NOTE: Selects explicit columns because `*` leaves no Project at all
over the right table scan, which a lookup
+ // join requires (see RelToPlanNodeConverter#convertLogicalJoin). That
case is covered in JoinPlans.json.
String query = "EXPLAIN PLAN FOR\n"
+ "SELECT /*+ joinOptions(join_strategy='lookup') */ \n"
- + "* FROM a\n"
+ + "a.col1, b.col2 FROM a\n"
+ "JOIN b\n"
+ "ON a.col1 = b.col1\n"
+ "WHERE a.col1 = 1;\n";
@@ -579,11 +581,14 @@ public class QueryCompilationTest extends
QueryEnvironmentTestBase {
//@formatter:off
assertEquals(explain,
"Execution Plan\n"
- + "LogicalJoin(condition=[=($0, $9)], joinType=[inner])\n"
- + " PinotLogicalExchange(distribution=[single])\n"
- + " LogicalFilter(condition=[=(CAST($0):INTEGER NOT NULL,
1)])\n"
- + " PinotLogicalTableScan(table=[[default, a]])\n"
- + " PinotLogicalTableScan(table=[[default, b]])\n");
+ + "LogicalProject(col1=[$0], col2=[$2])\n"
+ + " LogicalJoin(condition=[=($0, $1)], joinType=[inner])\n"
+ + " PinotLogicalExchange(distribution=[single])\n"
+ + " LogicalProject(col1=[$0])\n"
+ + " LogicalFilter(condition=[=(CAST($0):INTEGER NOT NULL,
1)])\n"
+ + " PinotLogicalTableScan(table=[[default, a]])\n"
+ + " LogicalProject(col1=[$0], col2=[$1])\n"
+ + " PinotLogicalTableScan(table=[[default, b]])\n");
//@formatter:on
}
diff --git
a/pinot-query-planner/src/test/java/org/apache/pinot/query/queries/ResourceBasedQueryPlansTest.java
b/pinot-query-planner/src/test/java/org/apache/pinot/query/queries/ResourceBasedQueryPlansTest.java
index 33f5977bc02..28bd8af259a 100644
---
a/pinot-query-planner/src/test/java/org/apache/pinot/query/queries/ResourceBasedQueryPlansTest.java
+++
b/pinot-query-planner/src/test/java/org/apache/pinot/query/queries/ResourceBasedQueryPlansTest.java
@@ -71,8 +71,6 @@ public class ResourceBasedQueryPlansTest extends
QueryEnvironmentTestBase {
try {
long requestId = RANDOM_REQUEST_ID_GEN.nextLong();
_queryEnvironment.explainQuery(query, requestId);
- String queryWithoutExplainPlan = query.replaceFirst(EXPLAIN_REGEX, "");
- _queryEnvironment.planQuery(queryWithoutExplainPlan);
Assert.fail("Query compilation should have failed with exception message
pattern: " + expectedException);
} catch (Exception e) {
if (expectedException == null) {
diff --git a/pinot-query-planner/src/test/resources/queries/JoinPlans.json
b/pinot-query-planner/src/test/resources/queries/JoinPlans.json
index a96520741ea..77a234e39f7 100644
--- a/pinot-query-planner/src/test/resources/queries/JoinPlans.json
+++ b/pinot-query-planner/src/test/resources/queries/JoinPlans.json
@@ -1133,6 +1133,11 @@
"description": "Lookup join with transformation on right table joined
key",
"sql": "EXPLAIN PLAN FOR SELECT /*+ joinOptions(join_strategy =
'lookup') */ a.col1, b.col2 FROM a JOIN b ON a.col1 = upper(b.col1)",
"expectedException": "Right input for lookup join must be an
identifier.*"
+ },
+ {
+ "description": "Lookup join selecting all columns leaves no Project
over the right table scan",
+ "sql": "EXPLAIN PLAN FOR SELECT /*+ joinOptions(join_strategy =
'lookup') */ * FROM a JOIN b ON a.col1 = b.col1 WHERE a.col1 = 1",
+ "expectedException": "Right input for lookup join must be a Project.*"
}
]
}
diff --git
a/pinot-query-planner/src/test/resources/queries/ValidationErrorPlan.json
b/pinot-query-planner/src/test/resources/queries/ValidationErrorPlan.json
index 980f521586d..494f1cf138f 100644
--- a/pinot-query-planner/src/test/resources/queries/ValidationErrorPlan.json
+++ b/pinot-query-planner/src/test/resources/queries/ValidationErrorPlan.json
@@ -11,6 +11,11 @@
"sql": "EXPLAIN PLAN FOR SELECT SUM(a.col3) as sumCol3,
arrayToMv(e.mcol1), a.col2 FROM a JOIN e on a.col1=e.col1 GROUP BY
arrayToMv(e.mcol1), a.col2",
"expectedException": ".*'ArrayToMv' is not supported.*"
},
+ {
+ "description": "arrayToMV validation error 2 (physical optimizer)",
+ "sql": "SET usePhysicalOptimizer=true; EXPLAIN PLAN FOR SELECT
SUM(a.col3) as sumCol3, arrayToMv(e.mcol1), a.col2 FROM a JOIN e on
a.col1=e.col1 GROUP BY arrayToMv(e.mcol1), a.col2",
+ "expectedException": ".*'ArrayToMv' is not supported.*"
+ },
{
"description": "Select * with negative limit -1",
"sql": "EXPLAIN PLAN FOR SELECT * FROM d LIMIT -1",
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
