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