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 >>>>> >>>>> >>> >>> >>
