Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24657 )
Change subject: IMPALA-13534: Implement runtime filters on CTEs ...................................................................... Patch Set 13: (3 comments) PS13: I looked at the FE changes and cte-consumer-node.cc in detail and skimmed the rest of the BE; the plan changes I checked around the CTE consumers rather than line by line. Both points from PS2 look resolved to me - the producer is now walked once via visitedCTEProducers_, and replacing finalizeRuntimeFilter() with the blocked-filter set does what I was after: q04 now shows RF001[min_max] landing on four different CTE consumers instead of stopping at the first one. Three questions below, none of them objections. http://gerrit.cloudera.org:8080/#/c/24657/13/be/src/exec/cte-consumer-node.cc File be/src/exec/cte-consumer-node.cc: http://gerrit.cloudera.org:8080/#/c/24657/13/be/src/exec/cte-consumer-node.cc@260 PS13, Line 260: FilterRowBatch(output_batch); In the non-passthrough path this runs after MaterializeBatch(), so rows the filter rejects have already been copied into the freshly allocated tuple buffer, while scanners evaluate their filters before materializing. The filter exprs are bound to the consumer's output tuple, so moving the evaluation earlier clearly isn't free - is that something for a follow-up, or is the extra copy cheap enough here that it doesn't matter? http://gerrit.cloudera.org:8080/#/c/24657/13/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java: http://gerrit.cloudera.org:8080/#/c/24657/13/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@997 PS13, Line 997: arrangeRuntimeFilters(childNode); Now that this always recurses into the children, it descends into the CTE producer once per consumer. Both arrange methods are plain sorts, so repeating them is harmless, but it is the same repeated-producer walk that generateFiltersRecursive() guards with visitedCTEProducers_. Would it be worth reusing that set here, or is the cost small enough not to bother? http://gerrit.cloudera.org:8080/#/c/24657/13/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@1480 PS13, Line 1480: if (filter.getType() == TRuntimeFilterType.MIN_MAX && !enable_overlap_filter) { enableOverlapFilter() describes Parquet overlap pruning: it reads parquet_read_statistics and the minmax_filter_* options, and on the scan path a min/max filter is kept only if tryToComputeOverlapPredicate() also succeeds. A CTE consumer applies min/max row by row through FilterContext::Eval() over batches from the LocalExchanger, where no Parquet statistics are involved, so with parquet_read_statistics=false these filters quietly disappear from consumers for a reason that doesn't apply to them. You mentioned overlap filter support isn't finished yet - is a criterion not tied to Parquet what you have in mind, or is this deliberately conservative until the rest lands? -- To view, visit http://gerrit.cloudera.org:8080/24657 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic877fb590187826f828da6a27bf274465c381e8e Gerrit-Change-Number: 24657 Gerrit-PatchSet: 13 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Tue, 18 Aug 2026 10:47:11 +0000 Gerrit-HasComments: Yes
