XieJiann commented on code in PR #24973:
URL: https://github.com/apache/doris/pull/24973#discussion_r1339576672


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/FilterEstimation.java:
##########
@@ -83,48 +84,51 @@ public FilterEstimation(Set<Slot> aggSlots) {
     public Statistics estimate(Expression expression, Statistics statistics) {
         // For a comparison predicate, only when it's left side is a slot and 
right side is a literal, we would
         // consider is a valid predicate.
-        return expression.accept(this, new EstimationContext(statistics));
+        Statistics stats = expression.accept(this, new 
EstimationContext(statistics));
+        stats.enforceValid();
+        return stats;
     }
 
     @Override
     public Statistics visit(Expression expr, EstimationContext context) {
-        return context.statistics.withSel(DEFAULT_INEQUALITY_COEFFICIENT);
+        return context.statistics.withSel(DEFAULT_INEQUALITY_COEFFICIENT, 
false);
     }
 
     @Override
     public Statistics visitCompoundPredicate(CompoundPredicate predicate, 
EstimationContext context) {
         Expression leftExpr = predicate.child(0);
         Expression rightExpr = predicate.child(1);
         Statistics leftStats = leftExpr.accept(this, context);
-        Statistics andStats = rightExpr.accept(new FilterEstimation(),
+        Statistics andStats = rightExpr.accept(this,
                 new EstimationContext(leftStats));
         if (predicate instanceof And) {
             return andStats;
         } else if (predicate instanceof Or) {
             Statistics rightStats = rightExpr.accept(this, context);
             double rowCount = leftStats.getRowCount() + 
rightStats.getRowCount() - andStats.getRowCount();
-            Statistics orStats = context.statistics.withRowCount(rowCount);
-            for (Map.Entry<Expression, ColumnStatistic> entry : 
orStats.columnStatistics().entrySet()) {
-                ColumnStatistic leftColStats = 
leftStats.findColumnStatistics(entry.getKey());
-                ColumnStatistic rightColStats = 
rightStats.findColumnStatistics(entry.getKey());
-                ColumnStatisticBuilder estimatedColStatsBuilder = new 
ColumnStatisticBuilder(entry.getValue());
-                if (leftColStats.minValue <= rightColStats.minValue) {
-                    
estimatedColStatsBuilder.setMinValue(leftColStats.minValue);
-                    estimatedColStatsBuilder.setMinExpr(leftColStats.minExpr);
-                } else {
-                    
estimatedColStatsBuilder.setMinValue(rightColStats.minValue);
-                    estimatedColStatsBuilder.setMinExpr(rightColStats.minExpr);
-                }
-                if (leftColStats.maxValue >= rightColStats.maxValue) {
-                    
estimatedColStatsBuilder.setMaxValue(leftColStats.maxValue);
-                    estimatedColStatsBuilder.setMaxExpr(leftColStats.maxExpr);
-                } else {
-                    
estimatedColStatsBuilder.setMaxValue(rightColStats.maxValue);
-                    estimatedColStatsBuilder.setMaxExpr(rightColStats.maxExpr);
+            Statistics orStats = context.statistics.setRowCount(rowCount);
+            Set<Slot> leftInputSlots = leftExpr.getInputSlots();
+            Set<Slot> rightInputSlots = rightExpr.getInputSlots();
+            for (Slot slot : context.keyColumns) {
+                if (leftInputSlots.contains(slot) && 
rightInputSlots.contains(slot)) {
+                    ColumnStatistic leftColStats = 
leftStats.findColumnStatistics(slot);
+                    ColumnStatistic rightColStats = 
rightStats.findColumnStatistics(slot);
+                    StatisticRange leftRange = 
StatisticRange.from(leftColStats, slot.getDataType());
+                    StatisticRange rightRange = 
StatisticRange.from(rightColStats, slot.getDataType());
+                    StatisticRange union = leftRange.union(rightRange);
+                    ColumnStatisticBuilder colBuilder = new 
ColumnStatisticBuilder(
+                            context.statistics.findColumnStatistics(slot));
+                    
colBuilder.setMinValue(union.getLow()).setMinExpr(union.getLowExpr())
+                            
.setMaxValue(union.getHigh()).setMaxExpr(union.getHighExpr())
+                            .setNdv(union.getDistinctValues());
+                    orStats.addColumnStats(slot, colBuilder.build());
                 }
             }
             return orStats;
         }
+        // should not come here
+        Preconditions.checkArgument(false, "unsupported compound operator: "
+                + predicate.getClass().getName() + " in " + predicate.toSql());

Review Comment:
   Maybe throw a runtime exception is better



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/FilterEstimation.java:
##########
@@ -83,48 +84,51 @@ public FilterEstimation(Set<Slot> aggSlots) {
     public Statistics estimate(Expression expression, Statistics statistics) {
         // For a comparison predicate, only when it's left side is a slot and 
right side is a literal, we would
         // consider is a valid predicate.
-        return expression.accept(this, new EstimationContext(statistics));
+        Statistics stats = expression.accept(this, new 
EstimationContext(statistics));
+        stats.enforceValid();
+        return stats;
     }
 
     @Override
     public Statistics visit(Expression expr, EstimationContext context) {
-        return context.statistics.withSel(DEFAULT_INEQUALITY_COEFFICIENT);
+        return context.statistics.withSel(DEFAULT_INEQUALITY_COEFFICIENT, 
false);
     }
 
     @Override
     public Statistics visitCompoundPredicate(CompoundPredicate predicate, 
EstimationContext context) {
         Expression leftExpr = predicate.child(0);
         Expression rightExpr = predicate.child(1);
         Statistics leftStats = leftExpr.accept(this, context);
-        Statistics andStats = rightExpr.accept(new FilterEstimation(),
+        Statistics andStats = rightExpr.accept(this,
                 new EstimationContext(leftStats));
         if (predicate instanceof And) {
             return andStats;
         } else if (predicate instanceof Or) {
             Statistics rightStats = rightExpr.accept(this, context);
             double rowCount = leftStats.getRowCount() + 
rightStats.getRowCount() - andStats.getRowCount();
-            Statistics orStats = context.statistics.withRowCount(rowCount);
-            for (Map.Entry<Expression, ColumnStatistic> entry : 
orStats.columnStatistics().entrySet()) {
-                ColumnStatistic leftColStats = 
leftStats.findColumnStatistics(entry.getKey());
-                ColumnStatistic rightColStats = 
rightStats.findColumnStatistics(entry.getKey());
-                ColumnStatisticBuilder estimatedColStatsBuilder = new 
ColumnStatisticBuilder(entry.getValue());
-                if (leftColStats.minValue <= rightColStats.minValue) {
-                    
estimatedColStatsBuilder.setMinValue(leftColStats.minValue);
-                    estimatedColStatsBuilder.setMinExpr(leftColStats.minExpr);
-                } else {
-                    
estimatedColStatsBuilder.setMinValue(rightColStats.minValue);
-                    estimatedColStatsBuilder.setMinExpr(rightColStats.minExpr);
-                }
-                if (leftColStats.maxValue >= rightColStats.maxValue) {
-                    
estimatedColStatsBuilder.setMaxValue(leftColStats.maxValue);
-                    estimatedColStatsBuilder.setMaxExpr(leftColStats.maxExpr);
-                } else {
-                    
estimatedColStatsBuilder.setMaxValue(rightColStats.maxValue);
-                    estimatedColStatsBuilder.setMaxExpr(rightColStats.maxExpr);
+            Statistics orStats = context.statistics.setRowCount(rowCount);
+            Set<Slot> leftInputSlots = leftExpr.getInputSlots();
+            Set<Slot> rightInputSlots = rightExpr.getInputSlots();
+            for (Slot slot : context.keyColumns) {
+                if (leftInputSlots.contains(slot) && 
rightInputSlots.contains(slot)) {
+                    ColumnStatistic leftColStats = 
leftStats.findColumnStatistics(slot);
+                    ColumnStatistic rightColStats = 
rightStats.findColumnStatistics(slot);
+                    StatisticRange leftRange = 
StatisticRange.from(leftColStats, slot.getDataType());
+                    StatisticRange rightRange = 
StatisticRange.from(rightColStats, slot.getDataType());
+                    StatisticRange union = leftRange.union(rightRange);
+                    ColumnStatisticBuilder colBuilder = new 
ColumnStatisticBuilder(
+                            context.statistics.findColumnStatistics(slot));
+                    
colBuilder.setMinValue(union.getLow()).setMinExpr(union.getLowExpr())
+                            
.setMaxValue(union.getHigh()).setMaxExpr(union.getHighExpr())
+                            .setNdv(union.getDistinctValues());
+                    orStats.addColumnStats(slot, colBuilder.build());
                 }
             }
             return orStats;
         }
+        // should not come here
+        Preconditions.checkArgument(false, "unsupported compound operator: "
+                + predicate.getClass().getName() + " in " + predicate.toSql());

Review Comment:
   Maybe throwing a runtime exception is better



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to