[GitHub] [doris] Kikyou1997 commented on a diff in pull request #13883: [feature-wip](nereids) Statistics persistence

[GitBox]

Kikyou1997 commented on code in PR #13883:
URL: https://github.com/apache/doris/pull/13883#discussion_r1018096041


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/ExpressionEstimation.java:
##########
@@ -38,94 +38,99 @@
 import org.apache.doris.nereids.trees.expressions.literal.Literal;
 import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
 import org.apache.doris.nereids.util.Utils;
-import org.apache.doris.statistics.ColumnStat;
+import org.apache.doris.statistics.ColumnStatistic;
+import org.apache.doris.statistics.ColumnStatisticBuilder;
 import org.apache.doris.statistics.StatsDeriveResult;
 
 import com.google.common.base.Preconditions;
 
 /**
  * Used to estimate for expressions that not producing boolean value.
  */
-public class ExpressionEstimation extends ExpressionVisitor<ColumnStat, 
StatsDeriveResult> {
+public class ExpressionEstimation extends ExpressionVisitor<ColumnStatistic, 
StatsDeriveResult> {
 
     private static ExpressionEstimation INSTANCE = new ExpressionEstimation();
 
     /**
      * returned columnStat is newly created or a copy of stats
      */
-    public static ColumnStat estimate(Expression expression, StatsDeriveResult 
stats) {
+    public static ColumnStatistic estimate(Expression expression, 
StatsDeriveResult stats) {
         return INSTANCE.visit(expression, stats);
     }
 
     @Override
-    public ColumnStat visit(Expression expr, StatsDeriveResult context) {
+    public ColumnStatistic visit(Expression expr, StatsDeriveResult context) {
         return expr.accept(this, context);
     }
 
     //TODO: case-when need to re-implemented
     @Override
-    public ColumnStat visitCaseWhen(CaseWhen caseWhen, StatsDeriveResult 
context) {
-        ColumnStat columnStat = new ColumnStat();
+    public ColumnStatistic visitCaseWhen(CaseWhen caseWhen, StatsDeriveResult 
context) {
+        ColumnStatisticBuilder columnStat = new ColumnStatisticBuilder();
         columnStat.setNdv(caseWhen.getWhenClauses().size() + 1);
-        columnStat.setSelectivity(1.0);
         columnStat.setMinValue(0);
         columnStat.setMaxValue(Double.MAX_VALUE);
         columnStat.setAvgSizeByte(8);
         columnStat.setNumNulls(0);
-        columnStat.setMaxSizeByte(8);
-        return columnStat;
+        return columnStat.createColumnStatistic();
     }
 
-    public ColumnStat visitCast(Cast cast, StatsDeriveResult context) {
+    public ColumnStatistic visitCast(Cast cast, StatsDeriveResult context) {
         return cast.child().accept(this, context);
     }
 
     @Override
-    public ColumnStat visitLiteral(Literal literal, StatsDeriveResult context) 
{
-        if 
(ColumnStat.MAX_MIN_UNSUPPORTED_TYPE.contains(literal.getDataType().toCatalogDataType()))
 {
-            return ColumnStat.UNKNOWN;
+    public ColumnStatistic visitLiteral(Literal literal, StatsDeriveResult 
context) {
+        if 
(ColumnStatistic.MAX_MIN_UNSUPPORTED_TYPE.contains(literal.getDataType().toCatalogDataType()))
 {
+            return ColumnStatistic.DEFAULT;
         }
         double literalVal = literal.getDouble();
-        ColumnStat columnStat = new ColumnStat();
+        ColumnStatisticBuilder columnStat = new ColumnStatisticBuilder();
         columnStat.setMaxValue(literalVal);
         columnStat.setMinValue(literalVal);
         columnStat.setNdv(1);
         columnStat.setNumNulls(1);
         columnStat.setAvgSizeByte(1);
-        return columnStat;
+        return columnStat.createColumnStatistic();
+
     }
 
     @Override
-    public ColumnStat visitSlotReference(SlotReference slotReference, 
StatsDeriveResult context) {
-        ColumnStat columnStat = context.getColumnStatsBySlot(slotReference);
+    public ColumnStatistic visitSlotReference(SlotReference slotReference, 
StatsDeriveResult context) {
+        ColumnStatistic columnStat = 
context.getColumnStatsBySlotId(slotReference.getExprId());
         Preconditions.checkState(columnStat != null);
         return columnStat.copy();
     }
 
     @Override
-    public ColumnStat visitBinaryArithmetic(BinaryArithmetic binaryArithmetic, 
StatsDeriveResult context) {
-        ColumnStat leftColStats = binaryArithmetic.left().accept(this, 
context);
-        ColumnStat rightColStats = binaryArithmetic.right().accept(this, 
context);
-        double leftNdv = leftColStats.getNdv();
-        double rightNdv = rightColStats.getNdv();
+    public ColumnStatistic visitBinaryArithmetic(BinaryArithmetic 
binaryArithmetic, StatsDeriveResult context) {
+        ColumnStatistic leftColStats = binaryArithmetic.left().accept(this, 
context);
+        ColumnStatistic rightColStats = binaryArithmetic.right().accept(this, 
context);

Review Comment:
   Can't see the benefits for changing it 



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