Re: Problems on RC with HerdDB: was: [VOTE] Release apache-calcite-1.22.0 (release candidate 0)
I have attached a reproducer testcase for my problem. The problem is not in the planner itself but in the Sql to Rel conversion https://issues.apache.org/jira/browse/CALCITE-3826 I am sorry I am not able to open Calcite with IntelliJ nor NetBeans. I can push the producer to a github repo if you prefer, it is a simpler JUnit based test case Hope this helps Enrico Il giorno mar 25 feb 2020 alle ore 14:50 Enrico Olivelli ha scritto: > > Il giorno mar 25 feb 2020 alle ore 14:43 Vladimir Sitnikov > ha scritto: > > > > >Does any ring bell ? > > > > Is it related to [CALCITE-3672] Support implicit type coercion for insert > > and update ? > > https://issues.apache.org/jira/browse/CALCITE-3672 > > https://github.com/apache/calcite/pull/1720 > > > Vladimir, Danny, > yes I think that that patch can be the trigger of the regression. > I see that tests do not deal with bind variables. > > I wonder if there is some way to disable the new behaviour > otherwise I feel this can be a real blocker for the release. > > > > > > > >I am now trying to install IntelliJ, but it won't be so immediate > > > > AFAIK it should be seamless, so please follow up if you run into issues. > > I am able to open the project, I just have to get used to IntelliJ, no > problem. > > Enrico > > > > > Vladimir
Re: Problems on RC with HerdDB: was: [VOTE] Release apache-calcite-1.22.0 (release candidate 0)
Il giorno mar 25 feb 2020 alle ore 14:43 Vladimir Sitnikov ha scritto: > > >Does any ring bell ? > > Is it related to [CALCITE-3672] Support implicit type coercion for insert > and update ? > https://issues.apache.org/jira/browse/CALCITE-3672 > https://github.com/apache/calcite/pull/1720 Vladimir, Danny, yes I think that that patch can be the trigger of the regression. I see that tests do not deal with bind variables. I wonder if there is some way to disable the new behaviour otherwise I feel this can be a real blocker for the release. > > >I am now trying to install IntelliJ, but it won't be so immediate > > AFAIK it should be seamless, so please follow up if you run into issues. I am able to open the project, I just have to get used to IntelliJ, no problem. Enrico > > Vladimir
Re: Problems on RC with HerdDB: was: [VOTE] Release apache-calcite-1.22.0 (release candidate 0)
>Does any ring bell ? Is it related to [CALCITE-3672] Support implicit type coercion for insert and update ? https://issues.apache.org/jira/browse/CALCITE-3672 https://github.com/apache/calcite/pull/1720 >I am now trying to install IntelliJ, but it won't be so immediate AFAIK it should be seamless, so please follow up if you run into issues. Vladimir
Re: Problems on RC with HerdDB: was: [VOTE] Release apache-calcite-1.22.0 (release candidate 0)
I have found another problem about EnumerableTableModify#getSourceExpressionList
It looks like it is not mapping correctly the expected datatypes of
bind variables in queries like UPDATE table set a=?,b=? where pk=?.
I am sorry I am not able to create easily a test case.
I see Calcite code switched to Gradle + Kotlin configuration, but
this is not supported by my IDE,
so I am in trouble in creating a test case on Calcite (I am now trying
to install IntelliJ, but it won't be so immediate)
You can see the full SQL here in this commit in my test branch here
https://github.com/diennea/herddb/pull/563/commits/157f927c9efe85cf7cac1370e1637b1c7ec46dff#diff-5d7594bc81ae0c92bbd33dee6c0d189aR2301
My case is the following:
Create a table:
CREATE TABLE t1 (
field0 int PRIMARY KEY,
field1 VARCHAR(10),
field2 VARCHAR(10),
field3 INT,
field4 INT,
field5 VARCHAR(10)
)
UPDATE t1 SET field3 =?, field2=?, field4=?, field5=? where field0=?
The Update maps to this Calcite plan:
EnumerableTableModify(table=[[tblspace1, ip]], operation=[UPDATE],
updateColumnList=[[field3, field2, field4, field5]],
sourceExpressionList=[[?0, ?1, ?2, ?3]], flattened=[true]): rowcount =
1.0, cumulative cost = {2.5 rows, 10.5 cpu, 0.0 io}, id = 62
EnumerableProject(field0=[$0], field1=[$1], field2=[$2],
field3=[$3], field4=[$4], field5=[$5], EXPR$0=[?0], EXPR$1=[?1],
EXPR$2=[?2], EXPR$3=[?3]): rowcount = 1.0, cumulative cost = {1.5
rows, 10.5 cpu, 0.0 io}, id = 61
EnumerableInterpreter: rowcount = 1.0, cumulative cost = {0.5
rows, 0.5 cpu, 0.0 io}, id = 60
BindableTableScan(table=[[tblspace1, ip]], filters=[[=($0,
?4)]]): rowcount = 1.0, cumulative cost = {0.005 rows, 0.01 cpu, 0.0
io}, id = 45
In particular the obeserved problem is:
- the updateColumnList field is: field3, field2, field4, field5
- but the bind variables have wrong type: VARCHAR, VARCHAR, INT, INT,
expecting INT VARCHAR, INT VARCHAR
even by changing the UPDATE command the types of bind variables stays the same,
and they are the in the same order as in the CREATE TABLE STATEMENT,
skipping the PK.
Does any ring bell ?
Enrico
Il giorno mar 25 feb 2020 alle ore 12:32 Enrico Olivelli
ha scritto:
>
> Danny,
> We have found all of our showstoppers:
> - We were not handling JoinInfo#nonEquiConditions (maybe it has been
> added recently)
> - EnumerableDefaults#nestedLoopJoinOptimized assumes that
> AbstractEnumerable#asEnumerator returns an enumeration from the
> beginning of the stream, this is fine, but our previous code expected
> a call to "reset".
>
> We are testing downstream applications
>
> Thank you
> Enrico
>
> Il giorno mar 25 feb 2020 alle ore 10:25 Enrico Olivelli
> ha scritto:
> >
> > The issue is indeed about JoinInfo#nonEquiConditions and non equi
> > joins in general.
> >
> > I will have an answer within a couple of days: that means all tests
> > passing in HerdDB and in a bunch of well known downstream applications
> > that use Joins extensively
> >
> > Thank you Danny !
> >
> > Enrico
> >
> >
> > Il giorno mar 25 feb 2020 alle ore 10:08 Danny Chan
> > ha scritto:
> > >
> > > Without a plan diff of each case, it’s hard to tell whether the change is
> > > expected or turned to be wrong.
> > >
> > > There are indeed some commits that modify the Calcite Enumerable joins.
> > >
> > > From the timeline you gave, maybe you can check there 2 commits:
> > >
> > >
> > > 1.
> > > https://github.com/apache/calcite/commit/820f6ab4965d79946e4144db8e33aeef98c3d2ff
> > > 2.
> > > https://github.com/apache/calcite/commit/10a5b8a89d319e6fed563e7e49518cfc960b93d6
> > >
> > > Both seem to be a result fix though ~
> > >
> > > Best,
> > > Danny Chan
> > > 在 2020年2月25日 +0800 PM2:44，Enrico Olivelli ，写道：
> > > > Danny,
> > > > This is our workbench
> > > >
> > > >
> > > > https://github.com/diennea/herddb/pull/563
> > > >
> > > > I don't have a Calcite error/stacktrace but simply join results are not
> > > > correct.
> > > >
> > > > Last know passed Travis build against Calcite SNAPSHOT is around 24th
> > > > January
> > > >
> > > > Enrico
> > > >
> > > > Il Mar 25 Feb 2020, 04:04 Danny Chan ha scritto:
> > > >
> > > > > Thanks for the check XXX, ~
> > > > >
> > > > > You are right, in CALCITE-3769, we have deprecated the TableScanRule,
> > > > > the
> > > > > RelBuilder#scan and sql-to-rel conversion would always invokes
> > > > > RelOptTable#toRel, so there expects to have some plan changes for the
> > > > > TableScan node.
> > > > >
> > > > > We have moved the physical logic from RelOptTable#toRel to
> > > > > EnumerableTableScanRule which would invokes RelOptTable#getExpression
> > > > > ->
> > > > > QueryableTable#getExpression, so for your case, returns null is the
> > > > > right
> > > > > way to go.
> > > > >
> > > > > As for the Join failure, I didn’t see the stack trace, can you give
> > > > > more
> > > > > details ?
> > > > >
> > > > > [1] https://issues.apache.org/jira/browse/CALCITE-3769
> > > > >
> > > > >
Re: Problems on RC with HerdDB: was: [VOTE] Release apache-calcite-1.22.0 (release candidate 0)
Danny, We have found all of our showstoppers: - We were not handling JoinInfo#nonEquiConditions (maybe it has been added recently) - EnumerableDefaults#nestedLoopJoinOptimized assumes that AbstractEnumerable#asEnumerator returns an enumeration from the beginning of the stream, this is fine, but our previous code expected a call to "reset". We are testing downstream applications Thank you Enrico Il giorno mar 25 feb 2020 alle ore 10:25 Enrico Olivelli ha scritto: > > The issue is indeed about JoinInfo#nonEquiConditions and non equi > joins in general. > > I will have an answer within a couple of days: that means all tests > passing in HerdDB and in a bunch of well known downstream applications > that use Joins extensively > > Thank you Danny ! > > Enrico > > > Il giorno mar 25 feb 2020 alle ore 10:08 Danny Chan > ha scritto: > > > > Without a plan diff of each case, it’s hard to tell whether the change is > > expected or turned to be wrong. > > > > There are indeed some commits that modify the Calcite Enumerable joins. > > > > From the timeline you gave, maybe you can check there 2 commits: > > > > > > 1. > > https://github.com/apache/calcite/commit/820f6ab4965d79946e4144db8e33aeef98c3d2ff > > 2. > > https://github.com/apache/calcite/commit/10a5b8a89d319e6fed563e7e49518cfc960b93d6 > > > > Both seem to be a result fix though ~ > > > > Best, > > Danny Chan > > 在 2020年2月25日 +0800 PM2:44，Enrico Olivelli ，写道： > > > Danny, > > > This is our workbench > > > > > > > > > https://github.com/diennea/herddb/pull/563 > > > > > > I don't have a Calcite error/stacktrace but simply join results are not > > > correct. > > > > > > Last know passed Travis build against Calcite SNAPSHOT is around 24th > > > January > > > > > > Enrico > > > > > > Il Mar 25 Feb 2020, 04:04 Danny Chan ha scritto: > > > > > > > Thanks for the check XXX, ~ > > > > > > > > You are right, in CALCITE-3769, we have deprecated the TableScanRule, > > > > the > > > > RelBuilder#scan and sql-to-rel conversion would always invokes > > > > RelOptTable#toRel, so there expects to have some plan changes for the > > > > TableScan node. > > > > > > > > We have moved the physical logic from RelOptTable#toRel to > > > > EnumerableTableScanRule which would invokes RelOptTable#getExpression -> > > > > QueryableTable#getExpression, so for your case, returns null is the > > > > right > > > > way to go. > > > > > > > > As for the Join failure, I didn’t see the stack trace, can you give more > > > > details ? > > > > > > > > [1] https://issues.apache.org/jira/browse/CALCITE-3769 > > > > > > > > Best, > > > > Danny Chan > > > > 在 2020年2月24日 +0800 PM9:55，Enrico Olivelli ，写道： > > > > > Danny, > > > > > We are testing HerdDB with 1.22.0rc0 tag and we are seeing problems > > > > > with > > > > Joins. > > > > > > > > > > We were still on 1.19.0 and in December we created a test branch > > > > > against current Calcite's master. > > > > > Unfortunately during the past few weeks we stopped checking > > > > > continuously that branch and we missed the commit id on Calcite that > > > > > introduced these failures. > > > > > > > > > > All failures are about JOIN conditions that seem not to be applied > > > > correctly. > > > > > > > > > > This is my test branch with the upgrade from 1.19 to 1.22.0rc0: > > > > > https://github.com/diennea/herddb/pull/563 > > > > > > > > > > We are investigating, hopefully is only a bug in our changes. > > > > > Since 1.19.0 the management of JOINs has been changed a lot in Calcite > > > > > so probably we missed something. > > > > > > > > > > I also had to implement QueryableTable#getExpression that wasn't > > > > > required before, I have implemented it with a "return null" > > > > > > > > > > This was the error: > > > > > java.lang.RuntimeException: Error while applying rule > > > > > EnumerableTableScanRule(in:NONE,out:ENUMERABLE), args > > > > > [rel#26:LogicalTableScan.NONE.[](table=[tblspace1, tsql])] > > > > > at > > > > org.apache.calcite.plan.volcano.VolcanoRuleCall.onMatch(VolcanoRuleCall.java:244) > > > > > at > > > > org.apache.calcite.plan.volcano.VolcanoPlanner.findBestExp(VolcanoPlanner.java:636) > > > > > at herddb.sql.CalcitePlanner.runPlanner(CalcitePlanner.java:523) > > > > > at herddb.sql.CalcitePlanner.translate(CalcitePlanner.java:291) > > > > > at herddb.core.RawSQLTest.cacheStatement(RawSQLTest.java:96) > > > > > at > > > > > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native > > > > > Method) > > > > > at > > > > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > > > > > at > > > > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > > > > > at java.base/java.lang.reflect.Method.invoke(Method.java:567) > > > > > at > > > > org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50) > > > > > at > > > >
