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