vlsi commented on a change in pull request #1840: [CALCITE-3753] Remove rule 
queue importance
URL: https://github.com/apache/calcite/pull/1840#discussion_r388579721
 
 

 ##########
 File path: core/src/test/resources/sql/sub-query.iq
 ##########
 @@ -831,7 +831,7 @@ EnumerableCalc(expr#0..3=[{inputs}], expr#4=[NOT($t2)], 
expr#5=[IS TRUE($t4)], e
     EnumerableLimit(fetch=[1])
       EnumerableSort(sort0=[$0], dir0=[DESC])
         EnumerableAggregate(group=[{0}], c=[COUNT()])
-          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[false], cs=[$t3])
+          EnumerableCalc(expr#0..2=[{inputs}], expr#3=[false], expr#4=[123], 
expr#5=[null:INTEGER], expr#6=[=($t4, $t5)], expr#7=[IS NULL($t5)], 
expr#8=[OR($t6, $t7)], cs=[$t3], $condition=[$t8])
 
 Review comment:
   OK. You propose a change. It results in generating bad plans, thus it 
introduces a technical regression. There's a technical justification for -1.
   
   I know cost model has awful lot of inconsistencies. However, it turns out 
that all those 100 tiny inconsistencies cancel each other, and Calcite manages 
to produce "sane" plans.
   Now you fix one or two such defects (which has good merit), however, the net 
result becomes that there are 98 inconsistencies in the cost model which **no 
longer** cancel each other.
   
   Calcite purpose is optimizer, and it is really sad to introduce regressions 
to the optimizer.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to