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
<eolive...@gmail.com> 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
> <eolive...@gmail.com> 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
> > <yuzhao....@gmail.com> 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 <eolive...@gmail.com>,写道:
> > > > 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 <yuzhao....@gmail.com> 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 <eolive...@gmail.com>,写道:
> > > > > > 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
> > > > > org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
> > > > > > at
> > > > > org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
> > > > > > at
> > > > > org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
> > > > > > at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
> > > > > > at
> > > > > org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
> > > > > > at
> > > > > org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
> > > > > > at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
> > > > > > at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
> > > > > > at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
> > > > > > at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
> > > > > > at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
> > > > > > at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
> > > > > > at
> > > > > org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
> > > > > > at
> > > > > org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
> > > > > > at
> > > > > org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
> > > > > > at
> > > > > org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
> > > > > > at
> > > > > org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
> > > > > > at
> > > > > org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
> > > > > > at
> > > > > org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
> > > > > > at
> > > > > org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
> > > > > > Caused by: java.lang.RuntimeException: getExpression is not 
> > > > > > implemented
> > > > > > at
> > > > > herddb.sql.CalcitePlanner$TableImpl.getExpression(CalcitePlanner.java:1377)
> > > > > > at
> > > > > org.apache.calcite.prepare.RelOptTableImpl.lambda$getClassExpressionFunction$2(RelOptTableImpl.java:165)
> > > > > > at
> > > > > org.apache.calcite.prepare.RelOptTableImpl.getExpression(RelOptTableImpl.java:214)
> > > > > > at
> > > > > org.apache.calcite.adapter.enumerable.EnumerableTableScanRule.convert(EnumerableTableScanRule.java:63)
> > > > > > at
> > > > > org.apache.calcite.rel.convert.ConverterRule.onMatch(ConverterRule.java:144)
> > > > > > at
> > > > > org.apache.calcite.plan.volcano.VolcanoRuleCall.onMatch(VolcanoRuleCall.java:217)
> > > > > >
> > > > > > Best regards
> > > > > >
> > > > > > Enrico
> > > > > >
> > > > > > Il giorno lun 24 feb 2020 alle ore 11:38 Danny Chan
> > > > > > <yuzhao....@gmail.com> ha scritto:
> > > > > > >
> > > > > > > Just to note that, I have updated the release note though, so the
> > > > > history.md has diff for
> > > > > > > tar.gz in
> > > > > https://dist.apache.org/repos/dist/dev/calcite/apache-calcite-1.22.0-rc0/
> > > > > and
> > > > > > > the source tag calcite-1.22.0.
> > > > > > >
> > > > > > > Best,
> > > > > > > Danny Chan
> > > > > > > 在 2020年2月24日 +0800 PM6:34,Danny Chan <yuzhao....@gmail.com>,写道:
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > I have created a build for Apache Calcite 1.22.0, release 
> > > > > > > > candidate
> > > > > 0.
> > > > > > > >
> > > > > > > > Thanks to everyone who has contributed to this release.
> > > > > > > > <Further details about release.> You can read the release notes 
> > > > > > > > here:
> > > > > > > >
> > > > > https://github.com/apache/calcite/blob/calcite-1.22.0/site/_docs/history.md
> > > > > > > >
> > > > > > > > The commit to be voted upon:
> > > > > > > >
> > > > > https://gitbox.apache.org/repos/asf?p=calcite.git;a=commit;h=c6d55f13e4a6c2f53c13474e3e86a5a21834f9ed
> > > > > > > >
> > > > > > > > Its hash is c6d55f13e4a6c2f53c13474e3e86a5a21834f9ed.
> > > > > > > >
> > > > > > > > The artifacts to be voted on are located here:
> > > > > > > >
> > > > > https://dist.apache.org/repos/dist/dev/calcite/apache-calcite-1.22.0-rc0/
> > > > > > > >
> > > > > > > > The hashes of the artifacts are as follows:
> > > > > > > > src.tar.gz.sha512
> > > > > > > >
> > > > > > > >
> > > > > 4169d0a5db187582f92f6e78044fab8ae60b049caaed5b85b5873b761a2ed9e396da227f4594cacec53e19b41e455c2dff267119eb5447be4e58b5ef0564ef02
> > > > > *apache-calcite-1.22.0-src.tar.gz
> > > > > > > >
> > > > > > > > A staged Maven repository is available for review at:
> > > > > > > >
> > > > > https://repository.apache.org/content/repositories/orgapachecalcite-1076
> > > > > > > >
> > > > > > > > Release artifacts are signed with the following key:
> > > > > > > > https://people.apache.org/keys/committer/danny0405.asc
> > > > > > > >
> > > > > > > > Please vote on releasing this package as Apache Calcite 1.22.0.
> > > > > > > >
> > > > > > > > The vote is open for the next 72 hours and passes if a majority 
> > > > > > > > of
> > > > > > > > at least three +1 PMC votes are cast.
> > > > > > > >
> > > > > > > > [ ] +1 Release this package as Apache Calcite X.Y.Z
> > > > > > > > [ ] 0 I don't feel strongly about it, but I'm okay with the 
> > > > > > > > release
> > > > > > > > [ ] -1 Do not release this package because...
> > > > > > > >
> > > > > > > >
> > > > > > > > Here is my vote:
> > > > > > > >
> > > > > > > > +1 (binding)
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Danny Chan
> > > > >

Reply via email to