github-actions[bot] commented on code in PR #63974:
URL: https://github.com/apache/doris/pull/63974#discussion_r3534163705
##########
regression-test/suites/query_p0/stats/query_stats_test.groovy:
##########
@@ -198,6 +198,63 @@ suite("query_stats_test") {
def joinResult = sql "show query stats from ${tbName} all"
assertTrue((joinResult[0][1] as int) >= 1)
+ // UNION ALL: both branches contribute queryHit for k1; WHERE on right
branch adds k2.filterHit.
+ sql "clean all query stats"
+ sql "select k1 from ${tbName} union all select k1 from ${tbName} where k2
= 100"
+ def unionStats = sql "show query stats from ${tbName}"
+ def uK1 = unionStats.find { it[0] == "k1" }
+ def uK2 = unionStats.find { it[0] == "k2" }
+ assertNotNull(uK1)
+ assertNotNull(uK2)
+ assertTrue((uK1[1] as int) >= 1, "k1: UNION ALL queryHit")
+ assertTrue((uK2[2] as int) >= 1, "k2: WHERE on right UNION branch
filterHit")
+
+ // HAVING: k1 queryHit from GROUP BY + SELECT, k2 queryHit from SUM input,
+ // k2 filterHit from HAVING SUM(k2) > -1000.
+ sql "clean all query stats"
+ sql "select k1, sum(k2) from ${tbName} group by k1 having sum(k2) > -1000"
+ def havingStats = sql "show query stats from ${tbName}"
+ def hvK1 = havingStats.find { it[0] == "k1" }
+ def hvK2 = havingStats.find { it[0] == "k2" }
+ assertNotNull(hvK1)
+ assertNotNull(hvK2)
+ assertTrue((hvK1[1] as int) >= 1, "k1: GROUP BY / SELECT queryHit")
+ assertTrue((hvK2[1] as int) >= 1, "k2: SUM aggregate input queryHit")
+ assertTrue((hvK2[2] as int) >= 1, "k2: HAVING SUM(k2) filterHit")
+
+ // CTE: consumer query records queryHit on k2 and filterHit on k1.
+ sql "clean all query stats"
+ sql """with cte as (select k2 from ${tbName} where k1 = 1) select k2 from
cte"""
Review Comment:
This CTE case only references `cte` once, so with the default
`inline_cte_referenced_threshold = 1` it is inlined instead of leaving a
materialized CTE consumer/producer in the physical plan. `CTEInline` only keeps
the CTE when `consumers.size() > inlineCTEReferencedThreshold`, so this
regression can pass through the ordinary inlined scan/filter path without
exercising the new `PhysicalCTEProducer` / `PhysicalCTEConsumer` handling in
`QueryStatsRecorder`. Please make this case force a real CTE consumer, for
example by setting the threshold to `0` around the case and restoring it, or by
referencing the CTE twice.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/query/QueryStatsRecorder.java:
##########
@@ -308,14 +373,38 @@ private static void walkPlan(Plan plan,
}
}
}
- // filterHit for JOIN ON conditions (hash equality and non-equality
predicates).
+ // filterHit for all JOIN ON conditions; mark conjuncts are a separate
field not included
+ // in hashJoinConjuncts or otherJoinConjuncts (IN/EXISTS subquery
semi-join predicates).
if (plan instanceof AbstractPhysicalJoin) {
AbstractPhysicalJoin<?, ?> join = (AbstractPhysicalJoin<?, ?>)
plan;
for (Expression conjunct : join.getHashJoinConjuncts()) {
- recordInputSlotsAsFilterHit(conjunct, exprIdToScan,
exprIdToColName, deltas);
+ recordInputSlotsAsFilterHit(conjunct, exprIdToScan,
exprIdToColName, deltas, aggOutputToInputSlots);
}
for (Expression conjunct : join.getOtherJoinConjuncts()) {
- recordInputSlotsAsFilterHit(conjunct, exprIdToScan,
exprIdToColName, deltas);
+ recordInputSlotsAsFilterHit(conjunct, exprIdToScan,
exprIdToColName, deltas, aggOutputToInputSlots);
+ }
+ for (Expression conjunct : join.getMarkJoinConjuncts()) {
+ recordInputSlotsAsFilterHit(conjunct, exprIdToScan,
exprIdToColName, deltas, aggOutputToInputSlots);
+ }
+ }
+ // UNION / INTERSECT / EXCEPT: record queryHit for each child's
contributing columns.
+ if (plan instanceof PhysicalSetOperation) {
+ recordSetOpChildrenOutputs(
+ ((PhysicalSetOperation) plan).getRegularChildrenOutputs(),
+ exprIdToScan, exprIdToColName, deltas);
+ }
+ // PhysicalRecursiveUnion extends PhysicalBinary (not
PhysicalSetOperation); handle its
+ // getRegularChildrenOutputs() explicitly. Recursive-case
(WorkTableReference) slots skipped.
+ if (plan instanceof PhysicalRecursiveUnion) {
+ recordSetOpChildrenOutputs(
+ ((PhysicalRecursiveUnion<?, ?>)
plan).getRegularChildrenOutputs(),
+ exprIdToScan, exprIdToColName, deltas);
+ }
+ // LATERAL VIEW / EXPLODE: record queryHit for the generator input
columns (e.g. the
+ // array column passed to EXPLODE). Generated output slots are
synthetic and skipped.
+ if (plan instanceof PhysicalGenerate) {
Review Comment:
`PhysicalGenerate` also owns real filter predicates via `getConjuncts()`,
but this branch only records the generator arguments as `queryHit`. For `JOIN
unnest(...) ... ON ...`, `LogicalPlanBuilder` stores the ON condition in the
generate node and `PhysicalPlanTranslator.visitPhysicalGenerate` sends those
conjuncts to the `TableFunctionNode`; there is no `PhysicalFilter` or join node
later that will record them. A query such as a table-function join with `ON
name = 'x'` will execute that predicate, but `name` is used only from
`PhysicalGenerate.getConjuncts()` and never gets `filterHit`. Please iterate
the generate conjuncts here and call `recordInputSlotsAsFilterHit(...,
aggOutputToInputSlots)`, and add a case with a table-function join `ON`
predicate.
--
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]
