You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Julian Hyde <jh...@apache.org> on 2015/07/21 20:04:30 UTC
ProjectMergeRule
ProjectMergeRule currently refuses to reduce identity projects if the
fields have different names.
For instance suppose you have a table Dept (deptno, name) and the algebra
2: Project($1 as X, $0 as Y)
1: Project($1, $0)
0: Scan(Dept)
Observe that if you combine projects #1 and #2 you end up with
3: Project($0 as X, $1 as Y)
0: Scan(Dept)
Although the new project (#3) is an identity, it renames the fields.
ProjectMergeRule will return the new project (#3), but it could return
Scan(Dept) (#0).
Does anyone think they will break if I make it return #0?
Julian
Re: ProjectMergeRule
Posted by Julian Hyde <jh...@apache.org>.
Yes, I see that quite often.
https://issues.apache.org/jira/browse/CALCITE-687 covers race
conditions in the test framework.
On Tue, Jul 21, 2015 at 12:30 PM, Vladimir Sitnikov
<si...@gmail.com> wrote:
> I did try using force=true, and:
> 1) I observed no Calcite test failures. That is strange, since I
> expected a least a test or two fail due to column names
> 2) Example CSV failed with some assert, however a couple of subsequent
> mvn install -rf :calcite-example-csv healed it somehow. So we might
> have some race condition in metadata inialization:
>
> testDateType(org.apache.calcite.test.CsvTest) Time elapsed: 0.709 sec
> <<< FAILURE!
>
> java.lang.AssertionError: no provider found (rel=HepRelVertex#171,
> m=interface org.apache.calcite.rel.metadata.BuiltInMetadata$Collation);
> a backstop provider is recommended
> at org.apache.calcite.rel.AbstractRelNode.metadata(AbstractRelNode.java:272)
> at org.apache.calcite.rel.metadata.RelMetadataQuery.collations(RelMetadataQuery.java:294)
> at org.apache.calcite.rel.metadata.RelMdCollation.calc(RelMdCollation.java:180)
> at org.apache.calcite.adapter.enumerable.EnumerableCalc$2.get(EnumerableCalc.java:94)
> at org.apache.calcite.adapter.enumerable.EnumerableCalc$2.get(EnumerableCalc.java:92)
> at org.apache.calcite.plan.RelTraitSet.replaceIfs(RelTraitSet.java:238)
> at org.apache.calcite.adapter.enumerable.EnumerableCalc.create(EnumerableCalc.java:91)
> at org.apache.calcite.adapter.enumerable.EnumerableFilterToCalcRule.onMatch(EnumerableFilterToCalcRule.java:47)
> at org.apache.calcite.plan.AbstractRelOptPlanner.fireRule(AbstractRelOptPlanner.java:326)
> at org.apache.calcite.plan.hep.HepPlanner.applyRule(HepPlanner.java:515)
> at org.apache.calcite.plan.hep.HepPlanner.applyRules(HepPlanner.java:393)
> at org.apache.calcite.plan.hep.HepPlanner.executeInstruction(HepPlanner.java:255)
> at org.apache.calcite.plan.hep.HepInstruction$RuleInstance.execute(HepInstruction.java:125)
> at org.apache.calcite.plan.hep.HepPlanner.executeProgram(HepPlanner.java:207)
> at org.apache.calcite.plan.hep.HepPlanner.findBestExp(HepPlanner.java:194)
> at org.apache.calcite.tools.Programs$2.run(Programs.java:198)
> at org.apache.calcite.tools.Programs$SequenceProgram.run(Programs.java:320)
> at org.apache.calcite.prepare.Prepare.optimize(Prepare.java:142)
> at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:280)
> at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:188)
> at org.apache.calcite.prepare.CalcitePrepareImpl.prepare2_(CalcitePrepareImpl.java:669)
> at org.apache.calcite.prepare.CalcitePrepareImpl.prepare_(CalcitePrepareImpl.java:570)
> at org.apache.calcite.prepare.CalcitePrepareImpl.prepareSql(CalcitePrepareImpl.java:539)
> at org.apache.calcite.jdbc.CalciteConnectionImpl.parseQuery(CalciteConnectionImpl.java:173)
> at org.apache.calcite.jdbc.CalciteMetaImpl.prepareAndExecute(CalciteMetaImpl.java:561)
> at org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:477)
> at org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:110)
> at org.apache.calcite.avatica.AvaticaStatement.executeQuery(AvaticaStatement.java:130)
> at org.apache.calcite.test.CsvTest.testDateType(CsvTest.java:343)
>
> Vladimir
Re: ProjectMergeRule
Posted by Vladimir Sitnikov <si...@gmail.com>.
I did try using force=true, and:
1) I observed no Calcite test failures. That is strange, since I
expected a least a test or two fail due to column names
2) Example CSV failed with some assert, however a couple of subsequent
mvn install -rf :calcite-example-csv healed it somehow. So we might
have some race condition in metadata inialization:
testDateType(org.apache.calcite.test.CsvTest) Time elapsed: 0.709 sec
<<< FAILURE!
java.lang.AssertionError: no provider found (rel=HepRelVertex#171,
m=interface org.apache.calcite.rel.metadata.BuiltInMetadata$Collation);
a backstop provider is recommended
at org.apache.calcite.rel.AbstractRelNode.metadata(AbstractRelNode.java:272)
at org.apache.calcite.rel.metadata.RelMetadataQuery.collations(RelMetadataQuery.java:294)
at org.apache.calcite.rel.metadata.RelMdCollation.calc(RelMdCollation.java:180)
at org.apache.calcite.adapter.enumerable.EnumerableCalc$2.get(EnumerableCalc.java:94)
at org.apache.calcite.adapter.enumerable.EnumerableCalc$2.get(EnumerableCalc.java:92)
at org.apache.calcite.plan.RelTraitSet.replaceIfs(RelTraitSet.java:238)
at org.apache.calcite.adapter.enumerable.EnumerableCalc.create(EnumerableCalc.java:91)
at org.apache.calcite.adapter.enumerable.EnumerableFilterToCalcRule.onMatch(EnumerableFilterToCalcRule.java:47)
at org.apache.calcite.plan.AbstractRelOptPlanner.fireRule(AbstractRelOptPlanner.java:326)
at org.apache.calcite.plan.hep.HepPlanner.applyRule(HepPlanner.java:515)
at org.apache.calcite.plan.hep.HepPlanner.applyRules(HepPlanner.java:393)
at org.apache.calcite.plan.hep.HepPlanner.executeInstruction(HepPlanner.java:255)
at org.apache.calcite.plan.hep.HepInstruction$RuleInstance.execute(HepInstruction.java:125)
at org.apache.calcite.plan.hep.HepPlanner.executeProgram(HepPlanner.java:207)
at org.apache.calcite.plan.hep.HepPlanner.findBestExp(HepPlanner.java:194)
at org.apache.calcite.tools.Programs$2.run(Programs.java:198)
at org.apache.calcite.tools.Programs$SequenceProgram.run(Programs.java:320)
at org.apache.calcite.prepare.Prepare.optimize(Prepare.java:142)
at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:280)
at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:188)
at org.apache.calcite.prepare.CalcitePrepareImpl.prepare2_(CalcitePrepareImpl.java:669)
at org.apache.calcite.prepare.CalcitePrepareImpl.prepare_(CalcitePrepareImpl.java:570)
at org.apache.calcite.prepare.CalcitePrepareImpl.prepareSql(CalcitePrepareImpl.java:539)
at org.apache.calcite.jdbc.CalciteConnectionImpl.parseQuery(CalciteConnectionImpl.java:173)
at org.apache.calcite.jdbc.CalciteMetaImpl.prepareAndExecute(CalciteMetaImpl.java:561)
at org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:477)
at org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:110)
at org.apache.calcite.avatica.AvaticaStatement.executeQuery(AvaticaStatement.java:130)
at org.apache.calcite.test.CsvTest.testDateType(CsvTest.java:343)
Vladimir
Re: ProjectMergeRule
Posted by Jinfeng Ni <ji...@gmail.com>.
Drill insert a top Project to ensure the final field names are what
specified in the query. So, in general, field renaming in the intermediate
project should not impact query execution, except for * column in a
schema-less table. For * column, Drill has to keep the original form, since
the expansion to regular columns is delayed to execution time. If query
planner rename * column to ColumnA, then semantically it would return
different result. ( One commit in Drill's forked Calcite is specifically
to keep the * column name in subquery de-correlation logic, otherwise it
would break Drill's query execution).
For ProjectMergeRule, since we have the option to turn on/off force mode,
probably it would not impact Drill for now.
For Drill 1.2.0 release, we are targeting to rebase our forked Calcite onto
Calcite 1.4.0-SNAPSHOT. I'm also thinking about how to refactor the logic
of * column handling in planner while doing the rebasing. But it would take
time.
On Tue, Jul 21, 2015 at 3:13 PM, Jacques Nadeau <ja...@apache.org> wrote:
> I'd love the other Drill guys to chime here on their thoughts. Your
> suggestion makes sense.
>
> I'm not sure that Drill will have a problem since we do a final tree
> rewrite to avoid the name inconsistency issue.
>
> On Tue, Jul 21, 2015 at 12:23 PM, Julian Hyde <jh...@apache.org> wrote:
>
> > Jacques,
> >
> > I can make the default instance (ProjectMergeRule.INSTANCE) have
> > force=true (currently it has force=false) and only remove a renaming
> > project if force=true. Then most people will get the benefit, but if
> > there is a problem you can switch Drill to using a custom instance.
> >
> > Also, this would be good reason to test Drill against a 1.4-SNAPSHOT
> > when it is posted.
> >
> > Julian
> >
> >
> > On Tue, Jul 21, 2015 at 11:18 AM, Jacques Nadeau <ja...@apache.org>
> > wrote:
> > > I'm a little nervous about that for Drill. Despite the goal to do full
> > > testing to make sure we weren't accidentally using field names
> anywhere,
> > we
> > > haven't yet gotten very far. We know we're not supposed to depend on
> > > anything but ordinal but as a name based system, it is likely something
> > > depends on something there.
> > >
> > > On Tue, Jul 21, 2015 at 11:04 AM, Julian Hyde <jh...@apache.org>
> wrote:
> > >
> > >> ProjectMergeRule currently refuses to reduce identity projects if the
> > >> fields have different names.
> > >>
> > >> For instance suppose you have a table Dept (deptno, name) and the
> > algebra
> > >>
> > >> 2: Project($1 as X, $0 as Y)
> > >> 1: Project($1, $0)
> > >> 0: Scan(Dept)
> > >>
> > >> Observe that if you combine projects #1 and #2 you end up with
> > >>
> > >> 3: Project($0 as X, $1 as Y)
> > >> 0: Scan(Dept)
> > >>
> > >> Although the new project (#3) is an identity, it renames the fields.
> > >> ProjectMergeRule will return the new project (#3), but it could return
> > >> Scan(Dept) (#0).
> > >>
> > >> Does anyone think they will break if I make it return #0?
> > >>
> > >> Julian
> > >>
> >
>
Re: ProjectMergeRule
Posted by Jacques Nadeau <ja...@apache.org>.
I'd love the other Drill guys to chime here on their thoughts. Your
suggestion makes sense.
I'm not sure that Drill will have a problem since we do a final tree
rewrite to avoid the name inconsistency issue.
On Tue, Jul 21, 2015 at 12:23 PM, Julian Hyde <jh...@apache.org> wrote:
> Jacques,
>
> I can make the default instance (ProjectMergeRule.INSTANCE) have
> force=true (currently it has force=false) and only remove a renaming
> project if force=true. Then most people will get the benefit, but if
> there is a problem you can switch Drill to using a custom instance.
>
> Also, this would be good reason to test Drill against a 1.4-SNAPSHOT
> when it is posted.
>
> Julian
>
>
> On Tue, Jul 21, 2015 at 11:18 AM, Jacques Nadeau <ja...@apache.org>
> wrote:
> > I'm a little nervous about that for Drill. Despite the goal to do full
> > testing to make sure we weren't accidentally using field names anywhere,
> we
> > haven't yet gotten very far. We know we're not supposed to depend on
> > anything but ordinal but as a name based system, it is likely something
> > depends on something there.
> >
> > On Tue, Jul 21, 2015 at 11:04 AM, Julian Hyde <jh...@apache.org> wrote:
> >
> >> ProjectMergeRule currently refuses to reduce identity projects if the
> >> fields have different names.
> >>
> >> For instance suppose you have a table Dept (deptno, name) and the
> algebra
> >>
> >> 2: Project($1 as X, $0 as Y)
> >> 1: Project($1, $0)
> >> 0: Scan(Dept)
> >>
> >> Observe that if you combine projects #1 and #2 you end up with
> >>
> >> 3: Project($0 as X, $1 as Y)
> >> 0: Scan(Dept)
> >>
> >> Although the new project (#3) is an identity, it renames the fields.
> >> ProjectMergeRule will return the new project (#3), but it could return
> >> Scan(Dept) (#0).
> >>
> >> Does anyone think they will break if I make it return #0?
> >>
> >> Julian
> >>
>
Re: ProjectMergeRule
Posted by Julian Hyde <jh...@apache.org>.
Jacques,
I can make the default instance (ProjectMergeRule.INSTANCE) have
force=true (currently it has force=false) and only remove a renaming
project if force=true. Then most people will get the benefit, but if
there is a problem you can switch Drill to using a custom instance.
Also, this would be good reason to test Drill against a 1.4-SNAPSHOT
when it is posted.
Julian
On Tue, Jul 21, 2015 at 11:18 AM, Jacques Nadeau <ja...@apache.org> wrote:
> I'm a little nervous about that for Drill. Despite the goal to do full
> testing to make sure we weren't accidentally using field names anywhere, we
> haven't yet gotten very far. We know we're not supposed to depend on
> anything but ordinal but as a name based system, it is likely something
> depends on something there.
>
> On Tue, Jul 21, 2015 at 11:04 AM, Julian Hyde <jh...@apache.org> wrote:
>
>> ProjectMergeRule currently refuses to reduce identity projects if the
>> fields have different names.
>>
>> For instance suppose you have a table Dept (deptno, name) and the algebra
>>
>> 2: Project($1 as X, $0 as Y)
>> 1: Project($1, $0)
>> 0: Scan(Dept)
>>
>> Observe that if you combine projects #1 and #2 you end up with
>>
>> 3: Project($0 as X, $1 as Y)
>> 0: Scan(Dept)
>>
>> Although the new project (#3) is an identity, it renames the fields.
>> ProjectMergeRule will return the new project (#3), but it could return
>> Scan(Dept) (#0).
>>
>> Does anyone think they will break if I make it return #0?
>>
>> Julian
>>
Re: ProjectMergeRule
Posted by Jacques Nadeau <ja...@apache.org>.
After much discussion last year, I'm entirely in support of Julian's
abstract goals. However, I'm concerned about the real world impact.
On Tue, Jul 21, 2015 at 12:09 PM, Julian Hyde <jh...@apache.org> wrote:
> Yes, I do think these "little" relational expressions can have a very
> significant impact. They may block optimizations, they increase size
> of the search space (which makes the rule queue longer, and generates
> more GC pressure).
>
> To be clear: it's perfectly fine if client programs (e.g the
> SqlToRelConverter) create these expressions. But the planner rules
> have to be ruthless in stripping them away. And the engine has to do a
> better job of ignoring them after they have been shown to be trivial.
>
> On Tue, Jul 21, 2015 at 11:55 AM, Vladimir Sitnikov
> <si...@gmail.com> wrote:
> > I'm not sure if "field names for very top project" play any difference.
> >
> > As far as I understand, the intention was to keep execution plan
> readable.
> >
> > Julian, can you please highlight why are you thinking of merging
> > "project that merely renames"?
> >
> > Does it block another optimization?
> > Does it expand search space much?
> > Vladimir
>
Re: ProjectMergeRule
Posted by Julian Hyde <jh...@apache.org>.
Yes, I do think these "little" relational expressions can have a very
significant impact. They may block optimizations, they increase size
of the search space (which makes the rule queue longer, and generates
more GC pressure).
To be clear: it's perfectly fine if client programs (e.g the
SqlToRelConverter) create these expressions. But the planner rules
have to be ruthless in stripping them away. And the engine has to do a
better job of ignoring them after they have been shown to be trivial.
On Tue, Jul 21, 2015 at 11:55 AM, Vladimir Sitnikov
<si...@gmail.com> wrote:
> I'm not sure if "field names for very top project" play any difference.
>
> As far as I understand, the intention was to keep execution plan readable.
>
> Julian, can you please highlight why are you thinking of merging
> "project that merely renames"?
>
> Does it block another optimization?
> Does it expand search space much?
> Vladimir
Re: ProjectMergeRule
Posted by Vladimir Sitnikov <si...@gmail.com>.
I'm not sure if "field names for very top project" play any difference.
As far as I understand, the intention was to keep execution plan readable.
Julian, can you please highlight why are you thinking of merging
"project that merely renames"?
Does it block another optimization?
Does it expand search space much?
Vladimir
Re: ProjectMergeRule
Posted by Jacques Nadeau <ja...@apache.org>.
I'm a little nervous about that for Drill. Despite the goal to do full
testing to make sure we weren't accidentally using field names anywhere, we
haven't yet gotten very far. We know we're not supposed to depend on
anything but ordinal but as a name based system, it is likely something
depends on something there.
On Tue, Jul 21, 2015 at 11:04 AM, Julian Hyde <jh...@apache.org> wrote:
> ProjectMergeRule currently refuses to reduce identity projects if the
> fields have different names.
>
> For instance suppose you have a table Dept (deptno, name) and the algebra
>
> 2: Project($1 as X, $0 as Y)
> 1: Project($1, $0)
> 0: Scan(Dept)
>
> Observe that if you combine projects #1 and #2 you end up with
>
> 3: Project($0 as X, $1 as Y)
> 0: Scan(Dept)
>
> Although the new project (#3) is an identity, it renames the fields.
> ProjectMergeRule will return the new project (#3), but it could return
> Scan(Dept) (#0).
>
> Does anyone think they will break if I make it return #0?
>
> Julian
>