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


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/FilterEstimation.java:
##########
@@ -121,11 +123,106 @@ public Statistics estimate(Expression expression, 
Statistics inputStats) {
         return outputStats;
     }
 
+    /**
+     * Applies predicates already accounted for by the scan row count to 
column statistics only. The returned
+     * statistics preserve the input row count and its provenance. 
Single-column constraints share one
+     * estimation basis per slot; relationships between columns do not 
independently bound either column.
+     */
+    public Statistics applyColumnConstraints(Set<Expression> conjuncts, 
Statistics inputStats) {
+        Map<Slot, List<Expression>> conjunctsBySlot = new LinkedHashMap<>();
+        for (Expression conjunct : conjuncts) {
+            Set<Slot> inputSlots = conjunct.getInputSlots();
+            if (inputSlots.size() != 1) {
+                // A multi-column relationship does not independently bound 
either column's NDV.
+                continue;
+            }
+            Slot slot = inputSlots.iterator().next();
+            conjunctsBySlot.computeIfAbsent(slot, ignored -> new 
ArrayList<>()).add(conjunct);
+        }
+
+        StatisticsBuilder constrainedBuilder = new 
StatisticsBuilder(inputStats);
+        for (Map.Entry<Slot, List<Expression>> entry : 
conjunctsBySlot.entrySet()) {
+            Slot slot = entry.getKey();
+            constrainedBuilder.putColumnStatistics(slot,
+                    estimateConstrainedColumn(slot, 
ExpressionUtils.and(entry.getValue()), inputStats));
+        }
+        return constrainedBuilder.build();
+    }
+
+    private ColumnStatistic estimateConstrainedColumn(Slot slot, Expression 
predicate, Statistics scanStats) {
+        ColumnStatistic currentColumnStats = 
scanStats.findColumnStatistics(slot);
+        Preconditions.checkState(currentColumnStats != null,
+                "an applied scan predicate must have statistics for slot: %s", 
slot);
+        double scanRowCount = scanStats.getRowCount();
+
+        // Estimate against the original count and uncapped NDV: the selected 
row count may be much smaller
+        // than the table-level NDV. Retain any finite bounds already 
established by partition metadata.
+        ColumnStatistic estimationColumnStats = currentColumnStats;
+        double estimationRowCount = scanRowCount;
+        if (currentColumnStats.getOriginal() != null) {
+            ColumnStatisticBuilder estimationColumnBuilder =
+                    new 
ColumnStatisticBuilder(currentColumnStats.getOriginal());
+            if (Double.isFinite(currentColumnStats.minValue)) {
+                
estimationColumnBuilder.setMinValue(currentColumnStats.minValue)
+                        .setMinExpr(currentColumnStats.minExpr);
+            }
+            if (Double.isFinite(currentColumnStats.maxValue)) {
+                
estimationColumnBuilder.setMaxValue(currentColumnStats.maxValue)
+                        .setMaxExpr(currentColumnStats.maxExpr);
+            }
+            estimationColumnStats = estimationColumnBuilder.build();
+            estimationRowCount = Math.max(estimationRowCount, 
estimationColumnStats.count);
+        }
+        Statistics estimationBasis = new StatisticsBuilder()
+                .setRowCount(estimationRowCount)
+                .putColumnStatistics(slot, estimationColumnStats)
+                .build();
+        // estimate couples the row count and column domain in one pass, but 
this path needs only
+        // the domain: the scan row count is already fixed by partition 
pruning. Run the shared
+        // per-predicate logic on the synthetic basis, then keep the 
constrained domain and
+        // discard the row count below. This reuses every predicate type's 
domain computation
+        // instead of duplicating a domain-only variant of the visitor.
+        Statistics estimated = estimate(predicate, estimationBasis);
+        double estimatedRowCount = estimated.getRowCount();
+        if (estimatedRowCount <= 0) {
+            // Stale column statistics may contradict the partition proof. 
They cannot invalidate the
+            // selected scan row count, so retain this column's existing 
statistics.
+            return currentColumnStats;

Review Comment:
   [P2] Keep the proven NULL domain when stale statistics estimate zero
   
   Consider a nullable partition key with a NULL-only LIST partition containing 
1000 reported rows, while cached column statistics predate that insert (100000 
rows, NDV10000, numNulls0):
   
   ```text
   Aggregate(group by p)
     Filter(p IS NULL)
       OlapScan(selected NULL partition; 1000 rows)
   ```
   
   Restoring the original count/NDV makes the `isOnBase` check in `visitIsNull` 
true, so the synthetic estimate returns zero using the stale NULL count. This 
return then restores NDV1000/numNulls0 even though the pruning proof guarantees 
every selected row is NULL. `estimateGroupByRowCount` estimates 1000 groups 
instead of one. Previously the selected-count basis took the non-base IS NULL 
path and retained NDV0. Keep the exact NULL-only constraint (NDV0, numNulls 
equal to selected rows) when the estimated cardinality contradicts this proof; 
the new IS NULL test uses positive numNulls and misses this fallback.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/FilterEstimation.java:
##########
@@ -121,11 +123,106 @@ public Statistics estimate(Expression expression, 
Statistics inputStats) {
         return outputStats;
     }
 
+    /**
+     * Applies predicates already accounted for by the scan row count to 
column statistics only. The returned
+     * statistics preserve the input row count and its provenance. 
Single-column constraints share one
+     * estimation basis per slot; relationships between columns do not 
independently bound either column.
+     */
+    public Statistics applyColumnConstraints(Set<Expression> conjuncts, 
Statistics inputStats) {
+        Map<Slot, List<Expression>> conjunctsBySlot = new LinkedHashMap<>();
+        for (Expression conjunct : conjuncts) {
+            Set<Slot> inputSlots = conjunct.getInputSlots();
+            if (inputSlots.size() != 1) {
+                // A multi-column relationship does not independently bound 
either column's NDV.
+                continue;
+            }
+            Slot slot = inputSlots.iterator().next();
+            conjunctsBySlot.computeIfAbsent(slot, ignored -> new 
ArrayList<>()).add(conjunct);
+        }
+
+        StatisticsBuilder constrainedBuilder = new 
StatisticsBuilder(inputStats);
+        for (Map.Entry<Slot, List<Expression>> entry : 
conjunctsBySlot.entrySet()) {
+            Slot slot = entry.getKey();
+            constrainedBuilder.putColumnStatistics(slot,
+                    estimateConstrainedColumn(slot, 
ExpressionUtils.and(entry.getValue()), inputStats));
+        }
+        return constrainedBuilder.build();
+    }
+
+    private ColumnStatistic estimateConstrainedColumn(Slot slot, Expression 
predicate, Statistics scanStats) {
+        ColumnStatistic currentColumnStats = 
scanStats.findColumnStatistics(slot);
+        Preconditions.checkState(currentColumnStats != null,
+                "an applied scan predicate must have statistics for slot: %s", 
slot);
+        double scanRowCount = scanStats.getRowCount();
+
+        // Estimate against the original count and uncapped NDV: the selected 
row count may be much smaller
+        // than the table-level NDV. Retain any finite bounds already 
established by partition metadata.
+        ColumnStatistic estimationColumnStats = currentColumnStats;
+        double estimationRowCount = scanRowCount;
+        if (currentColumnStats.getOriginal() != null) {
+            ColumnStatisticBuilder estimationColumnBuilder =
+                    new 
ColumnStatisticBuilder(currentColumnStats.getOriginal());
+            if (Double.isFinite(currentColumnStats.minValue)) {
+                
estimationColumnBuilder.setMinValue(currentColumnStats.minValue)
+                        .setMinExpr(currentColumnStats.minExpr);
+            }
+            if (Double.isFinite(currentColumnStats.maxValue)) {
+                
estimationColumnBuilder.setMaxValue(currentColumnStats.maxValue)
+                        .setMaxExpr(currentColumnStats.maxExpr);
+            }
+            estimationColumnStats = estimationColumnBuilder.build();
+            estimationRowCount = Math.max(estimationRowCount, 
estimationColumnStats.count);
+        }
+        Statistics estimationBasis = new StatisticsBuilder()
+                .setRowCount(estimationRowCount)
+                .putColumnStatistics(slot, estimationColumnStats)
+                .build();
+        // estimate couples the row count and column domain in one pass, but 
this path needs only
+        // the domain: the scan row count is already fixed by partition 
pruning. Run the shared
+        // per-predicate logic on the synthetic basis, then keep the 
constrained domain and
+        // discard the row count below. This reuses every predicate type's 
domain computation
+        // instead of duplicating a domain-only variant of the visitor.
+        Statistics estimated = estimate(predicate, estimationBasis);

Review Comment:
   [P2] Preserve range-only OR domains in column constraints
   
   Consider this reduced plan over LIST partitions for each `p` value from 0 
through 1000, with 500 rows per value:
   
   ```text
   Aggregate(group by p)
     Filter(p < 1 OR p > 999)
       OlapScan(selected p=0,p=1000; 1000 rows)
   ```
   
   This call restores a 500500-row, 1001-NDV basis, but the constant-range 
visitors never register `p` in `EstimationContext.keyColumns`. Consequently 
`visitOr` skips its column-domain union and returns the table NDV; the new 
restoration then clamps it to 1000. `estimateGroupByRowCount` therefore 
estimates 1000 groups although only two values survive, materially distorting 
aggregation/join costs. Previously normalization capped this NDV against the 
reduced OR row estimate; separating the row count exposes the missing domain 
update. Ensure range-constrained slots participate in the OR union before 
retaining the domain, and cover a range-only OR: the added equality/IS NULL OR 
test registers keys and misses this case.



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