Skip to content

Commit 42baaef

Browse files
normen662normen662
committed
updating tests and writing documentation
1 parent 5b0b266 commit 42baaef

File tree

18 files changed

+158
-80
lines changed

18 files changed

+158
-80
lines changed

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AggregateIndexExpansionVisitor.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,10 +361,14 @@ private static Map<String, BuiltInFunction<? extends Value>> computeAggregateMap
361361
return mapBuilder.build();
362362
}
363363

364+
public static boolean canBeRolledUp(@Nonnull final String indexType) {
365+
return rollUpAggregateMap.get().containsKey(indexType);
366+
}
367+
364368
@Nonnull
365-
public static Optional<AggregateValue> rollUpAggregateValueMaybe(@Nonnull final Index index, @Nonnull final Value argument) {
369+
public static Optional<AggregateValue> rollUpAggregateValueMaybe(@Nonnull final String indexType, @Nonnull final Value argument) {
366370
return Optional.ofNullable(rollUpAggregateMap.get()
367-
.get(index.getType()))
371+
.get(indexType))
368372
.map(fn -> (AggregateValue)fn.encapsulate(ImmutableList.of(argument)));
369373
}
370374

@@ -373,8 +377,8 @@ private static Map<String, BuiltInFunction<? extends Value>> computeRollUpAggreg
373377
final ImmutableMap.Builder<String, BuiltInFunction<? extends Value>> mapBuilder = ImmutableMap.builder();
374378
mapBuilder.put(IndexTypes.MAX_EVER_LONG, new NumericAggregationValue.MaxFn());
375379
mapBuilder.put(IndexTypes.MIN_EVER_LONG, new NumericAggregationValue.MinFn());
376-
// mapBuilder.put(IndexTypes.MAX_EVER_TUPLE, TODO);
377-
// mapBuilder.put(IndexTypes.MIN_EVER_TUPLE, TODO);
380+
mapBuilder.put(IndexTypes.MAX_EVER_TUPLE, new NumericAggregationValue.MaxFn());
381+
mapBuilder.put(IndexTypes.MIN_EVER_TUPLE, new NumericAggregationValue.MinFn());
378382
mapBuilder.put(IndexTypes.SUM, new NumericAggregationValue.SumFn());
379383
mapBuilder.put(IndexTypes.COUNT, new NumericAggregationValue.SumFn());
380384
mapBuilder.put(IndexTypes.COUNT_NOT_NULL, new NumericAggregationValue.SumFn());

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AggregateIndexMatchCandidate.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ public RecordQueryPlan toEquivalentPlan(@Nonnull final PartialMatch partialMatch
451451
}
452452

453453
final var rollUpAggregateValueOptional =
454-
AggregateIndexExpansionVisitor.rollUpAggregateValueMaybe(index,
454+
AggregateIndexExpansionVisitor.rollUpAggregateValueMaybe(index.getType(),
455455
aggregateAccessorValue);
456456

457457
final var aggregateIndexScanQuantifier =

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PlanningRuleSet.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import com.apple.foundationdb.record.query.plan.cascades.rules.ImplementIntersectionRule;
4040
import com.apple.foundationdb.record.query.plan.cascades.rules.ImplementNestedLoopJoinRule;
4141
import com.apple.foundationdb.record.query.plan.cascades.rules.ImplementRecursiveUnionRule;
42-
import com.apple.foundationdb.record.query.plan.cascades.rules.ImplementRecursiveUnionRule;
4342
import com.apple.foundationdb.record.query.plan.cascades.rules.ImplementSimpleSelectRule;
4443
import com.apple.foundationdb.record.query.plan.cascades.rules.ImplementStreamingAggregationRule;
4544
import com.apple.foundationdb.record.query.plan.cascades.rules.ImplementTableFunctionRule;

‎fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/GroupByExpression.java

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.apple.foundationdb.record.PlanSerializationContext;
2626
import com.apple.foundationdb.record.planprotos.PValue;
2727
import com.apple.foundationdb.record.query.expressions.Comparisons;
28+
import com.apple.foundationdb.record.query.plan.cascades.AggregateIndexExpansionVisitor;
2829
import com.apple.foundationdb.record.query.plan.cascades.AliasMap;
2930
import com.apple.foundationdb.record.query.plan.cascades.Column;
3031
import com.apple.foundationdb.record.query.plan.cascades.ComparisonRange;
@@ -340,7 +341,7 @@ public Iterable<MatchInfo> subsumedBy(@Nonnull final RelationalExpression candid
340341
if (otherAggregateValues.size() != 1) {
341342
return ImmutableList.of();
342343
}
343-
final var otherPrimitiveAggregateValue = Iterables.getOnlyElement(otherAggregateValues);
344+
final var otherPrimitiveAggregateValue = (IndexableAggregateValue)Iterables.getOnlyElement(otherAggregateValues);
344345
final var matchedAggregatesMapBuilder = ImmutableBiMap.<Value, Value>builder();
345346
final var unmatchedAggregatesMapBuilder =
346347
ImmutableBiMap.<CorrelationIdentifier, Value>builder();
@@ -378,6 +379,12 @@ public Iterable<MatchInfo> subsumedBy(@Nonnull final RelationalExpression candid
378379
final var matchedGroupingsMap = subsumedGroupingsResult.getMatchedGroupingsMap();
379380
final var rollUpToGroupingValues = subsumedGroupingsResult.getRollUpToValues();
380381

382+
if (rollUpToGroupingValues != null &&
383+
!AggregateIndexExpansionVisitor.canBeRolledUp(otherPrimitiveAggregateValue.getIndexTypeName())) {
384+
// We determined we need a roll up, but we cannot do it base on the aggregations.
385+
return ImmutableList.of();
386+
}
387+
381388
final var unmatchedTranslatedAggregateValueMap =
382389
unmatchedTranslatedAggregatesValueMapBuilder.buildKeepingLast();
383390
final var translatedResultValue = getResultValue().translateCorrelations(translationMap, true);
@@ -416,7 +423,7 @@ private SubsumedGroupingsResult groupingSubsumedBy(@Nonnull final Quantifier can
416423
@Nonnull final ValueEquivalence valueEquivalence,
417424
@Nonnull final EvaluationContext evaluationContext) {
418425
if (groupingValue == null && candidateGroupingValue == null) {
419-
return SubsumedGroupingsResult.withoutRollUp(alwaysTrue(), ImmutableBiMap.of());
426+
return SubsumedGroupingsResult.withoutRollUp(BooleanWithConstraint.alwaysTrue(), ImmutableBiMap.of());
420427
}
421428
if (candidateGroupingValue == null) {
422429
return SubsumedGroupingsResult.noSubsumption();
@@ -426,7 +433,7 @@ private SubsumedGroupingsResult groupingSubsumedBy(@Nonnull final Quantifier can
426433
final BiMap<Value, Value> matchedGroupingsMap;
427434
if (groupingValue != null) {
428435
final var translatedGroupingsValuesBuilder = ImmutableList.<Value>builder();
429-
final var matchedGroupingsMapBuilder = ImmutableBiMap.<Value, Value>builder();
436+
final var matchedGroupingsMapBuilder = ImmutableMap.<Value, Value>builder();
430437
final var groupingValues =
431438
Values.primitiveAccessorsForType(groupingValue.getResultType(), () -> groupingValue).stream()
432439
.map(primitiveGroupingValue -> primitiveGroupingValue.simplify(evaluationContext,
@@ -440,7 +447,14 @@ private SubsumedGroupingsResult groupingSubsumedBy(@Nonnull final Quantifier can
440447
matchedGroupingsMapBuilder.put(primitiveGroupingValue, translatedPrimitiveGroupingValue);
441448
}
442449
translatedGroupingValues = translatedGroupingsValuesBuilder.build();
443-
matchedGroupingsMap = matchedGroupingsMapBuilder.build();
450+
451+
//
452+
// We know that if there are duplicates, they will be on the query side. Immutable bi-maps do not support
453+
// duplicated keys at all while regular maps do. The simplest and also the cheapest solution is to just
454+
// use an immutable map builder (which then is de-duped when built) and then use that map to build the
455+
// bi-map.
456+
//
457+
matchedGroupingsMap = ImmutableBiMap.copyOf(matchedGroupingsMapBuilder.buildKeepingLast());
444458
} else {
445459
translatedGroupingValues = ImmutableList.of();
446460
matchedGroupingsMap = ImmutableBiMap.of();
@@ -476,7 +490,7 @@ private SubsumedGroupingsResult groupingSubsumedBy(@Nonnull final Quantifier can
476490
// 3. For each candidate grouping value in the set of (yet) unmatched candidate group values, try to find a
477491
// predicate that binds that groupingValue.
478492
//
479-
var booleanWithConstraint = alwaysTrue();
493+
var booleanWithConstraint = BooleanWithConstraint.alwaysTrue();
480494
for (final var translatedGroupingValue : translatedGroupingValuesSet) {
481495
var found = false;
482496

@@ -562,7 +576,8 @@ private SubsumedGroupingsResult groupingSubsumedBy(@Nonnull final Quantifier can
562576
}
563577

564578
if (!unmatchedCandidateValues.isEmpty()) {
565-
Verify.verify(candidateGroupingValues.size() > translatedGroupingValues.size());
579+
Verify.verify(candidateGroupingValues.size() > translatedGroupingValuesSet.size());
580+
566581
//
567582
// This is a potential roll-up case, but only if the query side's groupings are completely subsumed
568583
// by the prefix of the candidate side. Iterate up to the smaller query side's grouping values to
@@ -630,14 +645,7 @@ public Compensation compensate(@Nonnull final PartialMatch partialMatch,
630645

631646
final var childCompensation = childCompensationOptional.get();
632647

633-
if (childCompensation.isImpossible()
634-
// ||
635-
// //
636-
// // TODO This needs some improvement as GB a, b, c WHERE a= AND c= needs to reapply the
637-
// // predicate on c which is currently refused here.
638-
// //
639-
// childCompensation.isNeededForFiltering()
640-
) {
648+
if (childCompensation.isImpossible()) {
641649
//
642650
// Note that it may be better to just return the child compensation verbatim as that compensation
643651
// may be combinable with something else to make it possible while the statically impossible compensation
@@ -737,7 +745,7 @@ public static Value flattenedResults(@Nullable final Value groupingKeyValue,
737745
AliasMap.identitiesFor(rcv.getCorrelatedTo()), ImmutableSet.of());
738746
}
739747

740-
public static class UnmatchedAggregateValue extends AbstractValue implements Value.NonEvaluableValue, IndexableAggregateValue {
748+
public static class UnmatchedAggregateValue extends AbstractValue implements Value.NonEvaluableValue {
741749
@Nonnull
742750
private final CorrelationIdentifier unmatchedId;
743751

@@ -756,12 +764,6 @@ protected Iterable<? extends Value> computeChildren() {
756764
return ImmutableList.of();
757765
}
758766

759-
@Nonnull
760-
@Override
761-
public String getIndexTypeName() {
762-
throw new UnsupportedOperationException();
763-
}
764-
765767
@Nonnull
766768
@Override
767769
public ExplainTokensWithPrecedence explain(@Nonnull final Iterable<Supplier<ExplainTokensWithPrecedence>> explainSuppliers) {
@@ -838,7 +840,7 @@ public List<Value> getRollUpToValues() {
838840

839841
@Nonnull
840842
public static SubsumedGroupingsResult noSubsumption() {
841-
return of(falseValue(), ImmutableBiMap.of(), null);
843+
return of(BooleanWithConstraint.falseValue(), ImmutableBiMap.of(), null);
842844
}
843845

844846
@Nonnull

0 commit comments

Comments
 (0)