Thanks Julian, the ticket is open: https://issues.apache.org/jira/browse/CALCITE-2184
Given its size, I plan to propose the "fix" along with the PR with the new Spark adapter tests if that's ok. On 15 February 2018 at 19:46, Julian Hyde <jh...@apache.org> wrote: > Yes, log a JIRA case. Making RelSubset.copy return this, rather than > throw, seems pragmatic. > > Long-term I would like to get rid of copy, so we might reverse this change > at some point. But by that time, these tests will be enabled. > > Julian > > > > On Feb 14, 2018, at 4:04 PM, Alessandro Solimando < > alessandro.solima...@gmail.com> wrote: > > > > Hi, > > while preparing some additional unit tests for the spark adapter ( > > https://github.com/asolimando/calcite/tree/SPARK-TESTS) I have stumbled > > upon the issue several times. > > > > This is the list of the tests that in my opinion should be succeeding but > > are failing because of an invocation of *copy* method for *RelSubset* > class: > > - testFilterBetween > > - testFilterIsIn > > - testFilterTrue > > - testFilterFalse > > - testFilterOr > > - testFilterIsNotNull > > - testFilterIsNull > > > > As you can infer from the name, the common trait among them is the > presence > > of a "complex" filtering condition in the where clause. > > > > Just as a side note (not implying this is a solution), by replacing the > > exception throwing with "return this;" inside *RelSubset.copy, *the > > aforementioned tests pass. > > > > Can you please acknowledge the issue (if any) so I can open a ticket, and > > reference it in the "@Ignore" of those tests, so I can advance with the > PR? > > > > Best regards, > > Alessandro > > > > On 12 February 2018 at 09:56, Alessandro Solimando < > > alessandro.solima...@gmail.com> wrote: > > > >> Hello Julian, > >> If I got it right, trimming the query plan for unused fields is a > top-down > >> procedure removing any reference to unused fields in the subplan rooted > at > >> the considered tree node. > >> > >> This, in principle, can affect also those coming from elements of > >> *RelSubset*, independently from the fact that they are in an equivalence > >> class, and that their result is "immutable". The only source of problem > I > >> see, is that the very concept of *RelSubset* suggests a "global scope", > >> and updating it according to the contextual information of a specific > >> subplan could break its correctness (the relational expressions > composing > >> *RelSubset* would be fitting only some of original contexts in which > they > >> were originally equivalent). > >> > >> However, *trimUnusedFields*, in my example, tries to update the traits > of > >> RelSubset's elements. > >> > >> So, if *RelSubset* should be immutable (traits included), then the > >> *trimUnusedFields* method should never call *copy* on it, but it does, > >> and the exception is thrown. > >> > >> The fact that implementing copy for *RelSubset* as the identity (that > is, > >> simply returning "this", ignoring any modification to the traits) did > not > >> introduce any problem reinforces the immutability hypothesis. > >> > >> Is my understanding correct? > >> Given that the query looks legal, the problem looks "real". > >> If this is confirmed, how do you suggest to address it? > >> > >> On 12 February 2018 at 00:04, Julian Hyde <jh...@apache.org> wrote: > >> > >>> Can you tell me why you want to copy a RelSubset? > >>> > >>> A RelSubset is an equivalence class - a set of relational expressions > >>> that always return the same results. So if you made a copy you’d be > >>> creating another equivalent relational expression - that by definition > >>> should be in the original RelSubset. > >>> > >>>> On Feb 11, 2018, at 1:18 PM, Alessandro Solimando < > >>> alessandro.solima...@gmail.com> wrote: > >>>> > >>>> Hello community, > >>>> > >>>> I am adding a SparkAdapter test with the following query: > >>>> > >>>>> select * > >>>>> from *(values (1, 'a'), (2, 'b'), (3, 'b'), (4, 'c'), (2, 'c')) as > >>> t(x, y)* > >>>>> where x between 3 and 4 > >>>>> > >>>>> When executed, an exception is thrown (the full stack trace is at the > >>> end > >>>> of the email) in *copy* method in *RelSubset* class, while Calcite is > >>>> trying to get rid of unused terms (specifically, *trimUnusedFields* > >>> method > >>>> from *SqlToRelConverted* class). > >>>> > >>>> The signature of copy is as follows: public RelNode copy(RelTraitSet > >>>> traitSet, List<RelNode> inputs) > >>>> > >>>> First of all, I don't understand the reason for the > >>>> *UnsupportedOperationException* in the first place. Why a RelSubset > >>>> shouldn't be copied? > >>>> > >>>> Assuming that the functionality is simply missing, I have considered > two > >>>> alternatives for implementing it: > >>>> 1) copy as the identity function -> all Calcite tests pass, but I am > >>>> ignoring the *traitSet* parameter in this way, looks odd > >>>> 2) I have tried to build a new *RelSubset* by reusing the cluster and > >>> set > >>>> information from the object, and the trait argument of copy -> assert > >>>> traits.allSimple(); > >>>> fails in the constructor > >>>> > >>>> In my example, the trait "[1]" (ordering detected at tuple level on > the > >>> 2nd > >>>> component) is transformed into a composite trait "[[1]]", this makes > the > >>>> assertion fail. > >>>> While I know what a trait is, I don't understand what a composite one > >>> is. > >>>> Do you have a concrete example? > >>>> > >>>> So the problem here is the introduction of the composite trait, which > is > >>>> caused by the *replace* method in *trimUnusedFields*: > >>>> > >>>> if (isTrimUnusedFields()) { > >>>>>> final RelFieldTrimmer trimmer = newFieldTrimmer(); > >>>>>> final List<RelCollation> collations = > >>>>>> rootRel.getTraitSet().getTraits(RelCollationTraitDef.INSTANCE); > >>>>>> rootRel = trimmer.trim(rootRel); > >>>>>> if (!ordered > >>>>>> && collations != null > >>>>>> && !collations.isEmpty() > >>>>>> && !collations.equals(ImmutableList.of(RelCollations.EMPTY))) { > >>>>>> final RelTraitSet traitSet = rootRel.getTraitSet() > >>>>>> .replace(RelCollationTraitDef.INSTANCE, collations); > >>>>>> rootRel = rootRel.copy(traitSet, rootRel.getInputs()); > >>>>>> } > >>>>>> if (SQL2REL_LOGGER.isDebugEnabled()) { > >>>>>> SQL2REL_LOGGER.debug( > >>>>>> RelOptUtil.dumpPlan("Plan after trimming unused fields", > rootRel, > >>>>>> SqlExplainFormat.TEXT, SqlExplainLevel.EXPPLAN_ATTRIBUTES)); > >>>>>> } > >>>>>> } > >>>>> > >>>>> > >>>> It is also not clear to me what the first *if* is trying to accomplish > >>>> here. I mean, the traits are never modified here, so it really looks > >>> like > >>>> the only reason for calling *replace* is to apply whatever side effect > >>> this > >>>> method has (the conversion from traits to composite traits looks the > >>> only > >>>> one to me), and in the specific situations matching the if condition. > >>> Can > >>>> you clarify which scenario the if is handling? > >>>> > >>>> I would appreciate also a feedback on the implementation of copy as > >>>> identity. Is it correct for you? > >>>> Or do you suggest the second option by enforcing a flattening of > traits > >>>> before calling the constructor? > >>>> > >>>> Best regards, > >>>> Alessandro > >>>> > >>>> The full stack trace: > >>>> > >>>> java.lang.RuntimeException: With materializationsEnabled=false, > limit=0 > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert.assertQuery(CalciteAss > >>> ert.java:600) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert$AssertQuery.returns( > >>> CalciteAssert.java:1346) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert$AssertQuery.returns( > >>> CalciteAssert.java:1329) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert$AssertQuery.returnsUno > >>> rdered(CalciteAssert.java:1357) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.SparkAdapterTest.commonTester(SparkA > >>> dapterTest.java:93) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.SparkAdapterTest.testFilterBetween(S > >>> parkAdapterTest.java:460) > >>>>> > >>>>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > >>>>> > >>>>> at > >>>>>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAcce > >>> ssorImpl.java:62) > >>>>> > >>>>> at > >>>>>> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMe > >>> thodAccessorImpl.java:43) > >>>>> > >>>>> at java.lang.reflect.Method.invoke(Method.java:498) > >>>>> > >>>>> at > >>>>>> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall( > >>> FrameworkMethod.java:50) > >>>>> > >>>>> at > >>>>>> org.junit.internal.runners.model.ReflectiveCallable.run(Refl > >>> ectiveCallable.java:12) > >>>>> > >>>>> at > >>>>>> org.junit.runners.model.FrameworkMethod.invokeExplosively(Fr > >>> ameworkMethod.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(BlockJUnit > >>> 4ClassRunner.java:78) > >>>>> > >>>>> at > >>>>>> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit > >>> 4ClassRunner.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.junit.runner.JUnitCore.run(JUnitCore.java:137) > >>>>> > >>>>> at > >>>>>> com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs > >>> (JUnit4IdeaTestRunner.java:68) > >>>>> > >>>>> at > >>>>>> com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.star > >>> tRunnerWithArgs(IdeaTestRunner.java:47) > >>>>> > >>>>> at > >>>>>> com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsA > >>> ndStart(JUnitStarter.java:242) > >>>>> > >>>>> at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStart > >>> er.java:70) > >>>>> > >>>>> Caused by: java.sql.SQLException: Error while executing SQL "select * > >>>>> > >>>>> from (values (1, 'a'), (2, 'b'), (3, 'b'), (4, 'c'), (2, 'c')) as > t(x, > >>> y) > >>>>> > >>>>> where x between 3 and 4": null > >>>>> > >>>>> at org.apache.calcite.avatica.Helper.createException(Helper.java:56) > >>>>> > >>>>> at org.apache.calcite.avatica.Helper.createException(Helper.java:41) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.avatica.AvaticaStatement.executeInternal( > >>> AvaticaStatement.java:156) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.avatica.AvaticaStatement.executeQuery(Ava > >>> ticaStatement.java:218) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert.assertQuery(CalciteAss > >>> ert.java:568) > >>>>> > >>>>> ... 27 more > >>>>> > >>>>> Caused by: java.lang.UnsupportedOperationException > >>>>> > >>>>> at org.apache.calcite.plan.volcano.RelSubset.copy( > RelSubset.java:149) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.sql2rel.SqlToRelConverter.trimUnusedField > >>> s(SqlToRelConverter.java:517) > >>>>> > >>>>> at org.apache.calcite.prepare.Prepare.trimUnusedFields(Prepare. > >>> java:391) > >>>>> > >>>>> at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:304) > >>>>> > >>>>> at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:230) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.prepare.CalcitePrepareImpl.prepare2_(Calc > >>> itePrepareImpl.java:781) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.prepare.CalcitePrepareImpl.prepare_(Calci > >>> tePrepareImpl.java:640) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.prepare.CalcitePrepareImpl.prepareSql(Cal > >>> citePrepareImpl.java:610) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.jdbc.CalciteConnectionImpl.parseQuery(Cal > >>> citeConnectionImpl.java:221) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.jdbc.CalciteMetaImpl.prepareAndExecute(Ca > >>> lciteMetaImpl.java:603) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.avatica.AvaticaConnection.prepareAndExecu > >>> teInternal(AvaticaConnection.java:638) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.avatica.AvaticaStatement.executeInternal( > >>> AvaticaStatement.java:149) > >>>>> > >>>>> ... 29 more > >>>>> > >>>>> > >>>>>> java.lang.RuntimeException: exception while executing [select * > >>>>> > >>>>> from (values (1, 'a'), (2, 'b'), (3, 'b'), (4, 'c'), (2, 'c')) as > t(x, > >>> y) > >>>>> > >>>>> where x between 3 and 4] > >>>>> > >>>>> > >>>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert$AssertQuery.returns( > >>> CalciteAssert.java:1351) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert$AssertQuery.returns( > >>> CalciteAssert.java:1329) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert$AssertQuery.returnsUno > >>> rdered(CalciteAssert.java:1357) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.SparkAdapterTest.commonTester(SparkA > >>> dapterTest.java:93) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.SparkAdapterTest.testFilterBetween(S > >>> parkAdapterTest.java:460) > >>>>> > >>>>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > >>>>> > >>>>> at > >>>>>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAcce > >>> ssorImpl.java:62) > >>>>> > >>>>> at > >>>>>> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMe > >>> thodAccessorImpl.java:43) > >>>>> > >>>>> at java.lang.reflect.Method.invoke(Method.java:498) > >>>>> > >>>>> at > >>>>>> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall( > >>> FrameworkMethod.java:50) > >>>>> > >>>>> at > >>>>>> org.junit.internal.runners.model.ReflectiveCallable.run(Refl > >>> ectiveCallable.java:12) > >>>>> > >>>>> at > >>>>>> org.junit.runners.model.FrameworkMethod.invokeExplosively(Fr > >>> ameworkMethod.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(BlockJUnit > >>> 4ClassRunner.java:78) > >>>>> > >>>>> at > >>>>>> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit > >>> 4ClassRunner.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.junit.runner.JUnitCore.run(JUnitCore.java:137) > >>>>> > >>>>> at > >>>>>> com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs > >>> (JUnit4IdeaTestRunner.java:68) > >>>>> > >>>>> at > >>>>>> com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.star > >>> tRunnerWithArgs(IdeaTestRunner.java:47) > >>>>> > >>>>> at > >>>>>> com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsA > >>> ndStart(JUnitStarter.java:242) > >>>>> > >>>>> at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStart > >>> er.java:70) > >>>>> > >>>>> Caused by: java.lang.RuntimeException: With > >>> materializationsEnabled=false, > >>>>>> limit=0 > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert.assertQuery(CalciteAss > >>> ert.java:600) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert$AssertQuery.returns( > >>> CalciteAssert.java:1346) > >>>>> > >>>>> ... 26 more > >>>>> > >>>>> Caused by: java.sql.SQLException: Error while executing SQL "select * > >>>>> > >>>>> from (values (1, 'a'), (2, 'b'), (3, 'b'), (4, 'c'), (2, 'c')) as > t(x, > >>> y) > >>>>> > >>>>> where x between 3 and 4": null > >>>>> > >>>>> at org.apache.calcite.avatica.Helper.createException(Helper.java:56) > >>>>> > >>>>> at org.apache.calcite.avatica.Helper.createException(Helper.java:41) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.avatica.AvaticaStatement.executeInternal( > >>> AvaticaStatement.java:156) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.avatica.AvaticaStatement.executeQuery(Ava > >>> ticaStatement.java:218) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.test.CalciteAssert.assertQuery(CalciteAss > >>> ert.java:568) > >>>>> > >>>>> ... 27 more > >>>>> > >>>>> Caused by: java.lang.UnsupportedOperationException > >>>>> > >>>>> at org.apache.calcite.plan.volcano.RelSubset.copy( > RelSubset.java:149) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.sql2rel.SqlToRelConverter.trimUnusedField > >>> s(SqlToRelConverter.java:517) > >>>>> > >>>>> at org.apache.calcite.prepare.Prepare.trimUnusedFields(Prepare. > >>> java:391) > >>>>> > >>>>> at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:304) > >>>>> > >>>>> at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:230) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.prepare.CalcitePrepareImpl.prepare2_(Calc > >>> itePrepareImpl.java:781) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.prepare.CalcitePrepareImpl.prepare_(Calci > >>> tePrepareImpl.java:640) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.prepare.CalcitePrepareImpl.prepareSql(Cal > >>> citePrepareImpl.java:610) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.jdbc.CalciteConnectionImpl.parseQuery(Cal > >>> citeConnectionImpl.java:221) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.jdbc.CalciteMetaImpl.prepareAndExecute(Ca > >>> lciteMetaImpl.java:603) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.avatica.AvaticaConnection.prepareAndExecu > >>> teInternal(AvaticaConnection.java:638) > >>>>> > >>>>> at > >>>>>> org.apache.calcite.avatica.AvaticaStatement.executeInternal( > >>> AvaticaStatement.java:149) > >>>>> > >>>>> ... 29 more > >>>>> > >>>>> > >>> > >>> > >> > >
