AMashenkov commented on code in PR #3537:
URL: https://github.com/apache/ignite-3/pull/3537#discussion_r1547573774
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/agg/IgniteSortAggregateBase.java:
##########
@@ -77,22 +80,42 @@ default Pair<RelTraitSet, List<RelTraitSet>>
passThroughCollation(
RelTraitSet nodeTraits, List<RelTraitSet> inputTraits
) {
RelCollation required = TraitUtils.collation(nodeTraits);
- ImmutableBitSet requiredKeys = ImmutableBitSet.of(required.getKeys());
- RelCollation collation;
- if (getGroupSet().contains(requiredKeys)) {
- List<RelFieldCollation> newCollationFields = new
ArrayList<>(getGroupSet().cardinality());
- newCollationFields.addAll(required.getFieldCollations());
-
- ImmutableBitSet keysLeft = getGroupSet().except(requiredKeys);
+ if (getGroupSet().isEmpty()) {
+ return passThroughCollation(nodeTraits, inputTraits,
RelCollations.EMPTY); // Erase collation for a single group.
+ } else if (required.getFieldCollations().isEmpty()) {
+ return passThroughCollation(nodeTraits, inputTraits,
TraitUtils.createCollation(getGroupSet().asList())); // No match.
+ }
- keysLeft.forEach(fieldIdx ->
newCollationFields.add(TraitUtils.createFieldCollation(fieldIdx)));
+ ImmutableBitSet groupingColumns =
ImmutableBitSet.range(getGroupSet().cardinality());
+ IntList prefix = maxPrefix(required.getKeys(),
groupingColumns.asSet());
- collation = RelCollations.of(newCollationFields);
- } else {
- collation = TraitUtils.createCollation(getGroupSet().toList());
+ if (prefix.isEmpty()) {
+ return passThroughCollation(nodeTraits, inputTraits,
TraitUtils.createCollation(getGroupSet().asList())); // No match.
}
+ // Rearrange grouping columns to satisfy required collation (as much
as possible).
+ List<RelFieldCollation> newCollationColumns = new
ArrayList<>(getGroupSet().cardinality());
+ Mapping mapping =
Commons.trimmingMapping(groupingColumns.cardinality(), groupingColumns);
+
+ // Add required columns first.
+ prefix.intStream().map(mapping::getTarget)
+ .forEach(fieldIdx ->
newCollationColumns.add(TraitUtils.createFieldCollation(fieldIdx)));
+
+ // Then add missed grouping columns.
+ groupingColumns.asList().stream()
+ .filter(not(prefix::contains))
+ .map(mapping::getTarget)
+ .forEach(fieldIdx ->
newCollationColumns.add(TraitUtils.createFieldCollation(fieldIdx)));
+
+ return passThroughCollation(nodeTraits, inputTraits,
RelCollations.of(newCollationColumns));
+ }
+
+ private static Pair<RelTraitSet, List<RelTraitSet>> passThroughCollation(
+ RelTraitSet nodeTraits,
+ List<RelTraitSet> inputTraits,
+ RelCollation collation
+ ) {
return Pair.of(nodeTraits.replace(collation),
List.of(inputTraits.get(0).replace(collation)));
}
Review Comment:
Got it. Fixed.
Also, I've found I used default order (asc/desc) for prefix columns instead
of preserving desired collation.
Seems, we need more tests on collation propagation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
