[
https://issues.apache.org/jira/browse/CALCITE-606?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14359167#comment-14359167
]
Jinfeng Ni commented on CALCITE-606:
------------------------------------
I have two comments.
1. Seems SortRemoveRule is not required, in order to have withoutHack() run
successful.
I commented out SortRemoveRule in RULES_NO_HACK, and withoutHack() is still
successful.
If the trait propagation works properly, then I think SortRemovalRule is not
necessary; why do we have to insert SORT first, then rely on SortRemoveRule to
remove the unnecessary SORT later on?
2. If I understand correctly, the main reason that withoutHack() in the
modified testcase is successful, is because of the following (line 156)
{code}
@Override public Statistic getStatistic() {
return Statistics.of(100d, ImmutableList.<ImmutableBitSet>of(),
ImmutableList.of(COLLATION));
}
{code}
That is, the logical EnumerableTableScan now has COLLATION. That is different
from the original testcase, where COLLATION is only known when logical
TableScan is converted into physical Scan operator. Actually, that's the main
reason why we would like to have trait propagation work : some traits, (
RelCollation , partition/distribution etc ) are not known until in Physical
operators, and at the Physical planning stage, we want to have those traits
propagated properly, such that there would be no unnecessary operator (SORT, or
EXCHANGE) would be inserted into the plan.
Actually, If I remove COLLATION from logical TableScan, then withoutHack() will
fail with CanNotPlan exception, meaning RelCollation propagated is not coming
from the Physical TableScan, but from Logical TableScan.
Not sure whether Jacques thinks this patch meets his need or not.
> Fix Trait Propagation and add test case
> ---------------------------------------
>
> Key: CALCITE-606
> URL: https://issues.apache.org/jira/browse/CALCITE-606
> Project: Calcite
> Issue Type: Bug
> Reporter: Jacques Nadeau
> Assignee: Jacques Nadeau
> Fix For: next
>
>
> Per my email to the dev list, it seems like we need to have a Trait
> propagation test case. Some things also seem broken. Dev list email:
> http://mail-archives.apache.org/mod_mbox/incubator-calcite-dev/201503.mbox/browser
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
