You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/08/13 06:34:06 UTC

[GitHub] [doris] jackwener opened a new pull request, #11755: [fix](Nereids): fix and enable cost derive job

jackwener opened a new pull request, #11755:
URL: https://github.com/apache/doris/pull/11755

   # Proposed changes
   
   Issue Number: close #xxx
   
   ## Problem summary
   
   fix and enable cost derive job
   
   Add mock for statistic in computeScan().
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: 
       - [x] Yes
       - [ ] No
       - [ ] I don't know
   2. Has unit tests been added:
       - [ ] Yes
       - [ ] No
       - [x] No Need
   3. Has document been added or modified:
       - [ ] Yes
       - [ ] No
       - [x] No Need
   4. Does it need to update dependencies:
       - [ ] Yes
       - [x] No
   5. Are there any changes that cannot be rolled back:
       - [ ] Yes (If Yes, please explain WHY)
       - [x] No
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   
   


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] github-actions[bot] commented on pull request #11755: [fix](Nereids): fix and enable stats derive job

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11755:
URL: https://github.com/apache/doris/pull/11755#issuecomment-1217868122

   PR approved by anyone and no changes requested.


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] jackwener commented on a diff in pull request #11755: [fix](Nereids): fix and enable stats derive job

Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #11755:
URL: https://github.com/apache/doris/pull/11755#discussion_r947418995


##########
fe/fe-core/src/test/java/org/apache/doris/nereids/jobs/CostAndEnforcerJobTest.java:
##########
@@ -1,102 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one

Review Comment:
   It's redundant, other UT have test the whole progress.



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] morrySnow commented on a diff in pull request #11755: [fix](Nereids): fix and enable stats derive job

Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #11755:
URL: https://github.com/apache/doris/pull/11755#discussion_r947416955


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/FilterSelectivityCalculator.java:
##########
@@ -107,4 +114,9 @@ public Double visitLessThan(LessThan lessThan, Void context) {
         return DEFAULT_SELECTIVITY;
     }
 
+    @Override
+    public Double visitInPredicate(InPredicate inPredicate, Void context) {

Review Comment:
   maybe we need to let this class inherit ExpressionVisitor and then Override visit function like what u do, and remove all visitXXX function that do same thing with visit function 



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java:
##########
@@ -195,9 +196,12 @@ private StatsDeriveResult computeFilter(Filter filter) {
     //       3. Get NDV and column data size from StatisticManger, StatisticManager doesn't support it now.
     private StatsDeriveResult computeScan(Scan scan) {
         Table table = scan.getTable();
-        TableStats tableStats = Utils.execWithReturnVal(() ->
-                ConnectContext.get().getEnv().getStatisticsManager().getStatistics().getTableStats(table.getId())
-        );
+        TableStats tableStats = Utils.execWithReturnVal(() -> {
+            Statistics statistics = ConnectContext.get().getEnv().getStatisticsManager().getStatistics();
+            // TODO: tmp mock the table stats, after we support the table stats, we should remove this mock.
+            statistics.mockTableStatsWithRowCount(scan.getTable().getId(), 0);

Review Comment:
   u should use scan node to compute row count just like what original planner do rather than set row count to zero



##########
fe/fe-core/src/test/java/org/apache/doris/nereids/jobs/CostAndEnforcerJobTest.java:
##########
@@ -1,102 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one

Review Comment:
   why remove 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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] 924060929 merged pull request #11755: [fix](Nereids): fix and enable stats derive job

Posted by GitBox <gi...@apache.org>.
924060929 merged PR #11755:
URL: https://github.com/apache/doris/pull/11755


-- 
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-unsubscribe@doris.apache.org

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


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