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:
[email protected]
With regards,
Apache Git Services