[
https://issues.apache.org/jira/browse/CALCITE-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17100209#comment-17100209
]
Julian Hyde commented on CALCITE-3969:
--------------------------------------
I see that you fixed some genuine bugs in Mappings, and added tests. Thanks.
Please modify the javadoc of {{RelTrait.apply}} to fully describe the behavior.
When does the method throw? When does it return null?
Should there be a method {{RelTrait.canApply(TargetMapping)}}?
Please also review methods {{RelDistribution.apply}},
{{RexUtil.apply(TargetMapping, FieldCollation)}}, and
{{RexUtil.applyFields(TargetMapping, FieldCollation)}}, their javadoc, and how
they are used. I see a couple of pieces of code that calls these methods and
does not handle a null return correctly. Wrap in {{Objects.requireNonNull}} if
a null return is impossible.
In {{RelDistributionTest}} and {{RelCollationTest}} it would be more
straightforward to pass {{MAPPING_SOURCE_COUNT}} as an argument to
{{mapping()}}. Assign it to a variable {{n}} so that the code is not too
verbose.
> Method RelTrait.apply(Mappings.Mapping) throws exception when mapping doesn't
> cover collation or distribution keys
> ------------------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-3969
> URL: https://issues.apache.org/jira/browse/CALCITE-3969
> Project: Calcite
> Issue Type: Bug
> Components: core
> Reporter: Roman Kondakov
> Assignee: Roman Kondakov
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.23.0
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> Let's consider we have an input {{(id, name)}} ordered by {{id}} (i.e.
> collation == {{[0]}}). If we have a {{Project("name")}} on the top of this
> input and we apply project's mapping on the collation, we'll end up with
> exception:
> {noformat}
> java.lang.NullPointerException: at index 0
> at
> com.google.common.collect.ObjectArrays.checkElementNotNull(ObjectArrays.java:239)
> at
> com.google.common.collect.ObjectArrays.checkElementsNotNull(ObjectArrays.java:230)
> at
> com.google.common.collect.ObjectArrays.checkElementsNotNull(ObjectArrays.java:225)
> at
> com.google.common.collect.ImmutableList.construct(ImmutableList.java:281)
> at
> com.google.common.collect.ImmutableList.copyOf(ImmutableList.java:239)
> at org.apache.calcite.rel.RelCollations.of(RelCollations.java:69)
> at org.apache.calcite.rex.RexUtil.apply(RexUtil.java:1271)
> at
> org.apache.calcite.rel.RelCollationImpl.apply(RelCollationImpl.java:122)
> at
> org.apache.calcite.rel.RelCollationImpl.apply(RelCollationImpl.java:40)
> {noformat}
> This happens because the collation field {{id}} is not a part of the mapping.
> The same problem is with distribution trait when distribution keys are not
> covered by the mapping.
> Calcite should handle such situations gracefully. If it is not possible to
> deduce the collation/distribution after the mapping application, we should
> return:
> * {{EMPTY}} collation for {{RelCollation}} trait.
> * {{RANDOM_DISTRIBUTED}} distribution for {{RelDistribution}} trait.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
