github-actions[bot] commented on code in PR #66898:
URL: https://github.com/apache/doris/pull/66898#discussion_r4118344043


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/CatalogRelation.java:
##########
@@ -43,4 +43,16 @@ default CatalogRelation withOperativeSlots(Collection<Slot> 
operativeSlots) {
     default List<Slot> getOperativeSlots() {
         return ImmutableList.of();
     }
+
+    /**
+     * Whether {@link #getOperativeSlots()} has been derived for this relation.
+     *
+     * <p>An empty operative slot list is a legitimate derivation result: a 
relation that the query
+     * does not need any column of, e.g. the scan of {@code select count(*) 
from wide_table}. The
+     * derivation state therefore must not be read from the slot list being 
empty, since a relation
+     * whose slots have not been derived yet carries an empty list as well.
+     */
+    default boolean isOperativeSlotsDerived() {

Review Comment:
   [P2] Carry the derived state into physical OLAP scans. 
`LogicalOlapScanToPhysicalOlapScan` copies `operativeSlots`, but 
`PhysicalOlapScan` inherits this default `false`. A reachable reduced plan is 
`LogicalProject(66) -> PhysicalStorageLayerAggregate(COUNT, 
relation=PhysicalOlapScan(wide, operative=[]))`: `AggregateStrategies` creates 
the physical-only child group, `DeriveStatsJob` estimates it, and 
`visitPhysicalStorageLayerAggregate` delegates to the embedded scan. 
`getStatsNeededSlots` then expands a correct derived-empty result to every 
output column, so `select 66` from a wide DUP table still triggers all 
column-stat loads. This is independent of the logical-copy issue above; 
propagate/preserve the boolean through logical-to-physical conversion and cover 
the storage-aggregate path with a zero-load test.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCatalogRelation.java:
##########
@@ -76,6 +76,8 @@ public abstract class LogicalCatalogRelation extends 
LogicalRelation implements
      */
     protected final String tableAlias;
 
+    private boolean operativeSlotsDerived;

Review Comment:
   [P2] Preserve this state through logical scan copies. After the final 
`OperativeColumnDerive`, `Memo.init` creates a `GroupExpression` for every 
leaf; its constructor immediately calls `LogicalOlapScan.withGroupExpression`, 
which constructs a fresh scan carrying `operativeSlots` while this field 
returns to `false`. `Optimizer` then runs `DeriveStatsJob`, so 
`getStatsNeededSlots` treats even `count(*)` as not derived and loads every 
visible column. The same loss also occurs earlier in `SetPreAggStatus` before 
`SkewJoin`. Thus the ordinary CBO path still performs the all-column loads this 
PR is intended to remove. Make the derivation state immutable/carry it through 
every semantics-preserving `withXxx` method and add memo-entry/rewrite-order 
wide-table load-count coverage.



-- 
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]

Reply via email to