mihaibudiu commented on code in PR #4423:
URL: https://github.com/apache/calcite/pull/4423#discussion_r2152799929
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -4259,21 +4255,14 @@ public void checkOrderBy(final boolean desc,
.withHook(Hook.PLANNER, (Consumer<RelOptPlanner>) planner ->
planner.removeRule(ENUMERABLE_MINUS_RULE))
.explainContains(""
- + "PLAN=EnumerableCalc(expr#0..3=[{inputs}], expr#4=[0],
expr#5=[>($t2, $t4)], "
- + "expr#6=[=($t3, $t4)], expr#7=[AND($t5, $t6)],
proj#0..1=[{exprs}], "
- + "$condition=[$t7])\n"
- + " EnumerableAggregate(group=[{0, 1}], agg#0=[COUNT() FILTER
$2], agg#1=[COUNT() "
- + "FILTER $3])\n"
- + " EnumerableCalc(expr#0..2=[{inputs}], expr#3=[0],
expr#4=[=($t2, $t3)], "
- + "expr#5=[1], expr#6=[=($t2, $t5)], proj#0..1=[{exprs}],
$f2=[$t4], $f3=[$t6])\n"
+ + "PLAN=EnumerableCalc(expr#0..3=[{inputs}], expr#4=[0],
expr#5=[>($t2, $t4)], expr#6=[=($t3, $t4)], expr#7=[AND($t5, $t6)],
proj#0..1=[{exprs}], $condition=[$t7])\n"
Review Comment:
I agree that this formatting is better, but it makes it harder to diff
I think you should leave it this way, but perhaps you can comment on the PR
when the plan is the same
or perhaps have two commits: one which shows the changes, and another one
which reformats
##########
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableHashJoinTest.java:
##########
@@ -205,14 +202,12 @@ class EnumerableHashJoinTest {
.query(
"select e.empid, e.name, d.name as dept from emps e join depts d"
+ " on e.deptno=d.deptno and e.empid<150 and e.empid>d.deptno")
- .explainContains("EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0],
name=[$t2], "
- + "dept=[$t4])\n"
+ .explainContains(""
+ + "EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2],
dept=[$t4])\n"
+ " EnumerableHashJoin(condition=[AND(=($1, $3), >($0, $3))],
joinType=[inner])\n"
- + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=[150],
expr#6=[<($t0, $t5)], "
- + "proj#0..2=[{exprs}], $condition=[$t6])\n"
- + " EnumerableTableScan(table=[[s, emps]])\n"
- + " EnumerableCalc(expr#0..3=[{inputs}], proj#0..1=[{exprs}])\n"
- + " EnumerableTableScan(table=[[s, depts]])\n")
+ + " EnumerableCalc(expr#0..4=[{inputs}],
expr#5=[CAST($t0):INTEGER NOT NULL], expr#6=[150], expr#7=[<($t5, $t6)],
proj#0..2=[{exprs}], $condition=[$t7])\n"
+ + " EnumerableTableScan(table=[[s, emps]])\n
EnumerableCalc(expr#0..3=[{inputs}], proj#0..1=[{exprs}])\n"
Review Comment:
same here
##########
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -1310,7 +1311,9 @@ private static List<RexNode>
convertOperands(SqlRexContext cx,
SqlOperandTypeChecker.Consistency consistency, List<RelDataType> types) {
switch (consistency) {
case COMPARE:
- if (SqlTypeUtil.areSameFamily(types)) {
+ if (SqlTypeUtil.areSameFamily(types)
Review Comment:
so this was a problem of missing casts, not of missing optimizations
##########
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableHashJoinTest.java:
##########
@@ -125,14 +124,12 @@ class EnumerableHashJoinTest {
.query(
"select e.empid, e.name, d.name as dept from emps e right outer "
+ "join depts d on e.deptno=d.deptno and e.empid<150")
- .explainContains("EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], "
- + "name=[$t2], dept=[$t4])\n"
+ .explainContains(""
+ + "EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0], name=[$t2],
dept=[$t4])\n"
+ " EnumerableHashJoin(condition=[=($1, $3)], joinType=[right])\n"
- + " EnumerableCalc(expr#0..4=[{inputs}], expr#5=[150], "
- + "expr#6=[<($t0, $t5)], proj#0..2=[{exprs}], $condition=[$t6])\n"
- + " EnumerableTableScan(table=[[s, emps]])\n"
- + " EnumerableCalc(expr#0..3=[{inputs}], proj#0..1=[{exprs}])\n"
- + " EnumerableTableScan(table=[[s, depts]])\n")
+ + " EnumerableCalc(expr#0..4=[{inputs}],
expr#5=[CAST($t0):INTEGER NOT NULL], expr#6=[150], expr#7=[<($t5, $t6)],
proj#0..2=[{exprs}], $condition=[$t7])\n"
+ + " EnumerableTableScan(table=[[s, emps]])\n
EnumerableCalc(expr#0..3=[{inputs}], proj#0..1=[{exprs}])\n"
Review Comment:
if you are reformatting these plans you should start a new line after this
newline.
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -2928,16 +2926,14 @@ private void
checkNullableTimestamp(CalciteAssert.Config config) {
+ " then \"name\" end)) T\n"
+ "from \"hr\".\"emps\"";
final String plan = ""
- + " String case_when_value;\n"
- + " final org.apache.calcite.test.schemata.hr.Employee
current = (org.apache"
- + ".calcite.test.schemata.hr.Employee) inputEnumerator.current();\n"
- + " if (current.empid > current.deptno * 10) {\n"
+ + " String case_when_value;\n"
+ + " final org.apache.calcite.test.schemata.hr.Employee
current = (org.apache.calcite.test.schemata.hr.Employee)
inputEnumerator.current();\n"
+ + " if
(org.apache.calcite.runtime.SqlFunctions.toInt(org.apache.calcite.linq4j.tree.Primitive.integerCast(org.apache.calcite.linq4j.tree.Primitive.INT,
current.empid, java.math.RoundingMode.DOWN)) > current.deptno * 10) {\n"
Review Comment:
I wonder why the previous plan would compile
--
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]
