[
https://issues.apache.org/jira/browse/BEAM-3417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16390732#comment-16390732
]
Anton Kedin edited comment on BEAM-3417 at 3/12/18 8:10 PM:
------------------------------------------------------------
*What fails?*
[Assert in question is in in VolcanoPlanner
|https://github.com/apache/calcite/blob/9ab47c0000732ec99c3162954e1eb74eaa30cddf/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java#L546].
It checks whether [all traits are
simple|https://github.com/apache/calcite/blob/0938c7b6d767e3242874d87a30d9112512d9243a/core/src/main/java/org/apache/calcite/plan/RelTraitSet.java#L517]
by checking whether they're not instances of RelCompositeTrait.
*Why it fails?*
In our case, when it fails, traitSet.allSimple() has 2 traits. One is
BeamLogicalConvention (it's not a composite trait), and another is a
collation-related composite trait which causes the assertion to fail.
*Where does the composite trait come from?*
We specify the collation trait def in
[BeamQueryPlanner|https://github.com/apache/beam/blob/14b17ad574342a875c8f99278e18c605aa5b4bc3/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/planner/BeamQueryPlanner.java#L89]
before parsing. It then [gets replaced in
LogicalTableScan|https://github.com/apache/calcite/blob/914b5cfbf978e796afeaff7b780e268ed39d8ec5/core/src/main/java/org/apache/calcite/rel/logical/LogicalTableScan.java#L102]
with the [composite
trait|https://github.com/apache/calcite/blob/0938c7b6d767e3242874d87a30d9112512d9243a/core/src/main/java/org/apache/calcite/plan/RelTraitSet.java#L239]
which causes the failure.
*Why LogicalTableScan needs to do the collation magic?*
Dunno, it seems that it adds the statistics information to the collation trait
so that the engine can handle sorting correctly. It does so only when we ask it
to by adding the collation trait def.
*Why VolcanoPlanner doesn't like CompositeTraitSet in that part?*
Dunno.
*Do we need the collation trait def?*
Dunno.
*What do we do?*
If we can, it probably makes sense to replace LogicalTableScan rel with some
kind of BeamIllogicalPCollectionScan which doesn't do all the collation magic
or makes it configurable.
- (update) At the second look, this assertion seems to happen before the Rel
Replacement, so we probably won't be able to replace the logical table scan rel
with our own logic.
was (Author: kedin):
*What fails?*
[Assert in question is in in VolcanoPlanner
|https://github.com/apache/calcite/blob/9ab47c0000732ec99c3162954e1eb74eaa30cddf/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java#L546].
It checks whether [all traits are
simple|https://github.com/apache/calcite/blob/0938c7b6d767e3242874d87a30d9112512d9243a/core/src/main/java/org/apache/calcite/plan/RelTraitSet.java#L517]
by checking whether they're not instances of RelCompositeTrait.
*Why it fails?*
In our case, when it fails, traitSet.allSimple() has 2 traits. One is
BeamLogicalConvention (it's not a composite trait), and another is a
collation-related composite trait which causes the assertion to fail.
*Where does the composite trait come from?*
We specify the collation trait def in
[BeamQueryPlanner|https://github.com/apache/beam/blob/14b17ad574342a875c8f99278e18c605aa5b4bc3/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/planner/BeamQueryPlanner.java#L89]
before parsing. It then [gets replaced in
LogicalTableScan|https://github.com/apache/calcite/blob/914b5cfbf978e796afeaff7b780e268ed39d8ec5/core/src/main/java/org/apache/calcite/rel/logical/LogicalTableScan.java#L102]
with the [composite
trait|https://github.com/apache/calcite/blob/0938c7b6d767e3242874d87a30d9112512d9243a/core/src/main/java/org/apache/calcite/plan/RelTraitSet.java#L239]
which causes the failure.
*Why LogicalTableScan needs to do the collation magic?*
Dunno, it seems that it adds the statistics information to the collation trait
so that the engine can handle sorting correctly. It does so only when we ask it
to by adding the collation trait def.
*Why VolcanoPlanner doesn't like CompositeTraitSet in that part?*
Dunno.
*Do we need the collation trait def?*
Dunno.
*What do we do?*
If we can, it probably makes sense to replace LogicalTableScanRel with some
kind of BeamIllogicalPCollectionScan which doesn't do all the collation magic
or makes it configurable
> Fix Calcite assertions
> ----------------------
>
> Key: BEAM-3417
> URL: https://issues.apache.org/jira/browse/BEAM-3417
> Project: Beam
> Issue Type: Task
> Components: dsl-sql
> Reporter: Anton Kedin
> Priority: Major
>
> Currently we disable assertions in test for every project which depends on
> Beam SQL / Calcite. Otherwise it fails assertions when Calcite validates
> relational representation of the query. E.g. in projects which depend on Beam
> SQL / Calcite we have to specify
> {code:java|title=build.gradle}
> test {
> jvmArgs "-da"
> }
> {code}
> We need to either update our relational conversion logic or come up with some
> other solution so that we don't have to disable assertions globally. If it's
> an incorrect assertion in Calcite then we need to fix it there.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
