mihaibudiu commented on code in PR #3994:
URL: https://github.com/apache/calcite/pull/3994#discussion_r1837084269
##########
core/src/main/java/org/apache/calcite/util/Bug.java:
##########
@@ -220,6 +220,12 @@ public abstract class Bug {
* is fixed. */
public static final boolean CALCITE_6328_FIXED = false;
+ /** Whether
Review Comment:
The title of the commit that introduces this change is misleading: you are
using the convention used for a commit which *solves* an issue, but this commit
only works around the issue. So I think before merging the title of the commit
should be changed.
##########
core/src/test/java/org/apache/calcite/test/ScannableTableTest.java:
##########
@@ -278,9 +278,10 @@ public class ScannableTableTest {
final Table table = new BeatlesProjectableFilterableTable(buf, false);
final String explain = "PLAN="
+ "EnumerableAggregate(group=[{0}], C=[COUNT()])\n"
- + " EnumerableAggregate(group=[{0, 1}])\n"
- + " EnumerableInterpreter\n"
- + " BindableTableScan(table=[[s, beatles]], filters=[[=($2,
1940)]], projects=[[2, 0]])";
+ + " EnumerableCalc(expr#0=[{inputs}], expr#1=[1940], k=[$t1],
i=[$t0])\n"
Review Comment:
I personally find it very difficult to evaluate whether these plans are
correct by inspection (including the ones in XML files). Or even if they are
better in any way.
On the other hand this PR is all about plan quality.
So I will have to trust you that the plans are better than they used to be
before.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
