Re: copy method in RelSubset class
Yes, it’s usually better that a bug as is described in terms of behavior an end-user would see, and has a test case for that behavior, rather than talking about internals “method x should call method y”. > On Feb 16, 2018, at 11:42 AM, Alessandro Solimando > <alessandro.solima...@gmail.com> wrote: > > 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 expressio
Re: copy method in RelSubset class
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
[jira] [Created] (CALCITE-2183) invocation of copy method in RelSubset class
Alessandro Solimando created CALCITE-2183:
-
Summary: invocation of copy method in RelSubset class
Key: CALCITE-2183
URL: https://issues.apache.org/jira/browse/CALCITE-2183
Project: Calcite
Issue Type: Bug
Components: core
Affects Versions: 1.16.0
Reporter: Alessandro Solimando
Assignee: Julian Hyde
Consider the following query:
{code:sql}
select *
from (values (1, 'a'), (2, 'b'), (3, 'b'), (4, 'c'), (2, 'c')) as t(x, y)
where x between 3 and 4
{code}
When executed, an exception is thrown (the full stack trace is at the end) in
_copy_ method in _RelSubset_ class, as the method is not meant to be called.
This happens (in this particular case) while Calcite is trying to get rid of
unused terms (specifically, _trimUnusedFields_ method from _SqlToRelConverted_
class).
This is problematic as the trace of calls is legitimate, so the method should
be executable.
Complete stack trace documenting the issue:
{noformat}
java.lang.RuntimeException: With materializationsEnabled=false, limit=0
at
org.apache.calcite.test.CalciteAssert.assertQuery(CalciteAssert.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.returnsUnordered(CalciteAssert.java:1357)
at
org.apache.calcite.test.SparkAdapterTest.commonTester(SparkAdapterTest.java:93)
at
org.apache.calcite.test.SparkAdapterTest.testFilterBetween(SparkAdapterTest.java:460)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.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(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.junit.runner.JUnitCore.run(JUnitCore.java:137)
at
com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at
com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
at
com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
at
com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.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(AvaticaStatement.java:218)
at
org.apache.calcite.test.CalciteAssert.assertQuery(CalciteAssert.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.trimUnusedFields(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_(CalcitePrepareImpl.java:781)
at
org.apache.calcite.prepare.CalcitePrepareImpl.prepare_(CalcitePrepareImpl.java:640)
at
org.apache.calcite.prepare.CalcitePrepareImpl.prepareSql(CalcitePrepareImp
Re: copy method in RelSubset class
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 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
Re: copy method in RelSubset class
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 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 wh
Re: copy method in RelSubset class
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 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 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
Re: copy method in RelSubset class
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 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 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(CalciteAssert.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.returnsUnordered(CalciteAssert.java:1357)
>>
>> at
>>> org.apache.calcite.test.SparkAdapterTest.commonTester(SparkAdapterTest.java:93)
>>
>> at
>>> org.apache.calcite.test.SparkAdapterTest.testFilterBetween(SparkAdapterTest.java:460)
>>
>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>>
>> at
>>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>
>> at
>>> sun.reflect.DelegatingMethodAccessorImpl.invoke(Delegat
copy method in RelSubset class
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 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 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(CalciteAssert.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.returnsUnordered(CalciteAssert.java:1357)
>
> at
>> org.apache.calcite.test.SparkAdapterTest.commonTester(SparkAdapterTest.java:93)
>
> at
>> org.apache.calcite.test.SparkAdapterTest.testFilterBetween(SparkAdapterTest.java:460)
>
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>
> at
>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>
> at
>> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.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(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)
>
>
