924060929 commented on code in PR #68726:
URL: https://github.com/apache/doris/pull/68726#discussion_r4228580328
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -1301,6 +1316,37 @@ public PlanFragment visitPhysicalHashAggregate(
return inputPlanFragment;
}
+ /** Match only row-count aggregates over an unfiltered external file TVF.
*/
+ static Optional<PhysicalTVFRelation> countPushDownFileTvf(
+ PhysicalHashAggregate<? extends Plan> aggregate, SessionVariable
sessionVariable) {
+ Plan child = aggregate.child(0);
+ Plan tvfChild = child instanceof PhysicalProject && child.child(0)
instanceof PhysicalTVFRelation
Review Comment:
[P2] Please check the surviving Project expressions before enabling COUNT
pushdown.
This branch treats any `PhysicalProject -> PhysicalTVFRelation` as
transparent, but COUNT readers return synthetic column values while the
projection still executes. A concrete triggering plan is:
```text
COUNT(*)
Project(assert_true(id > 0, 'positive id') AS checked)
file TVF (Parquet/ORC, id = 1, 2, 3, 4)
```
`AssertTrue` implements `NoneMovableFunction`, and
`LogicalProject.pruneOutputs()` explicitly preserves such expressions even when
the upper COUNT does not reference them. `visitPhysicalProject()` attaches the
expression to the scan's `ProjectList`, and
`OperatorXBase::get_next_after_projects()` evaluates it after reading the block.
With the new COUNT marker, V2 `TableReader::_materialize_count_rows()` fills
integer columns with default zero values instead of reading the real IDs. The
retained assertion therefore sees `0 > 0` and fails although every input ID is
positive. An assertion accepting zero can conversely hide an error that should
have occurred. V1 metadata COUNT also emits synthetic columns through
`CountReader`.
Please gate Project traversal on its actual surviving expressions, including
multi-layer projections. At minimum, reject projects containing
`NoneMovableFunction`; add enabled/disabled pushdown regressions for both a
passing and a failing data-dependent assertion.
This finding is based on the FE-to-BE source path at `5b83d45`; I have not
run a cluster reproduction.
--
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]
