Skip to content

Commit 4119b41

Browse files
normen662normen662
committed
updating tests and writing documentation
1 parent 551180c commit 4119b41

File tree

18 files changed

+159
-83
lines changed

18 files changed

+159
-83
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
@@ -357,10 +357,14 @@ private static Map<String, BuiltInFunction<? extends Value>> computeAggregateMap
357357
return mapBuilder.build();
358358
}
359359

360+
public static boolean canBeRolledUp(@Nonnull final String indexType) {
361+
return rollUpAggregateMap.get().containsKey(indexType);
362+
}
363+
360364
@Nonnull
361-
public static Optional<AggregateValue> rollUpAggregateValueMaybe(@Nonnull final Index index, @Nonnull final Value argument) {
365+
public static Optional<AggregateValue> rollUpAggregateValueMaybe(@Nonnull final String indexType, @Nonnull final Value argument) {
362366
return Optional.ofNullable(rollUpAggregateMap.get()
363-
.get(index.getType()))
367+
.get(indexType))
364368
.map(fn -> (AggregateValue)fn.encapsulate(ImmutableList.of(argument)));
365369
}
366370

@@ -369,8 +373,8 @@ private static Map<String, BuiltInFunction<? extends Value>> computeRollUpAggreg
369373
final ImmutableMap.Builder<String, BuiltInFunction<? extends Value>> mapBuilder = ImmutableMap.builder();
370374
mapBuilder.put(IndexTypes.MAX_EVER_LONG, new NumericAggregationValue.MaxFn());
371375
mapBuilder.put(IndexTypes.MIN_EVER_LONG, new NumericAggregationValue.MinFn());
372-
// mapBuilder.put(IndexTypes.MAX_EVER_TUPLE, TODO);
373-
// mapBuilder.put(IndexTypes.MIN_EVER_TUPLE, TODO);
376+
mapBuilder.put(IndexTypes.MAX_EVER_TUPLE, new NumericAggregationValue.MaxFn());
377+
mapBuilder.put(IndexTypes.MIN_EVER_TUPLE, new NumericAggregationValue.MinFn());
374378
mapBuilder.put(IndexTypes.SUM, new NumericAggregationValue.SumFn());
375379
mapBuilder.put(IndexTypes.COUNT, new NumericAggregationValue.SumFn());
376380
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
@@ -436,7 +436,7 @@ public RecordQueryPlan toEquivalentPlan(@Nonnull final PartialMatch partialMatch
436436
}
437437

438438
final var rollUpAggregateValueOptional =
439-
AggregateIndexExpansionVisitor.rollUpAggregateValueMaybe(index,
439+
AggregateIndexExpansionVisitor.rollUpAggregateValueMaybe(index.getType(),
440440
aggregateAccessorValue);
441441

442442
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 & 25 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.BooleanWithConstraint;
3031
import com.apple.foundationdb.record.query.plan.cascades.Column;
@@ -85,9 +86,6 @@
8586
import java.util.function.BiFunction;
8687
import java.util.function.Supplier;
8788

88-
import static com.apple.foundationdb.record.query.plan.cascades.BooleanWithConstraint.alwaysTrue;
89-
import static com.apple.foundationdb.record.query.plan.cascades.BooleanWithConstraint.falseValue;
90-
9189
/**
9290
* A logical {@code group by} expression that represents grouping incoming tuples and aggregating each group.
9391
*/
@@ -338,7 +336,7 @@ public Iterable<MatchInfo> subsumedBy(@Nonnull final RelationalExpression candid
338336
if (otherAggregateValues.size() != 1) {
339337
return ImmutableList.of();
340338
}
341-
final var otherPrimitiveAggregateValue = Iterables.getOnlyElement(otherAggregateValues);
339+
final var otherPrimitiveAggregateValue = (IndexableAggregateValue)Iterables.getOnlyElement(otherAggregateValues);
342340
final var matchedAggregatesMapBuilder = ImmutableBiMap.<Value, Value>builder();
343341
final var unmatchedAggregatesMapBuilder =
344342
ImmutableBiMap.<CorrelationIdentifier, Value>builder();
@@ -376,6 +374,12 @@ public Iterable<MatchInfo> subsumedBy(@Nonnull final RelationalExpression candid
376374
final var matchedGroupingsMap = subsumedGroupingsResult.getMatchedGroupingsMap();
377375
final var rollUpToGroupingValues = subsumedGroupingsResult.getRollUpToValues();
378376

377+
if (rollUpToGroupingValues != null &&
378+
!AggregateIndexExpansionVisitor.canBeRolledUp(otherPrimitiveAggregateValue.getIndexTypeName())) {
379+
// We determined we need a roll up, but we cannot do it base on the aggregations.
380+
return ImmutableList.of();
381+
}
382+
379383
final var unmatchedTranslatedAggregateValueMap =
380384
unmatchedTranslatedAggregatesValueMapBuilder.buildKeepingLast();
381385
final var translatedResultValue = getResultValue().translateCorrelations(translationMap, true);
@@ -413,7 +417,7 @@ private SubsumedGroupingsResult subsumedGroupings(@Nonnull final Quantifier cand
413417
@Nonnull final TranslationMap translationMap,
414418
@Nonnull final ValueEquivalence valueEquivalence) {
415419
if (groupingValue == null && candidateGroupingValue == null) {
416-
return SubsumedGroupingsResult.withoutRollUp(alwaysTrue(), ImmutableBiMap.of());
420+
return SubsumedGroupingsResult.withoutRollUp(BooleanWithConstraint.alwaysTrue(), ImmutableBiMap.of());
417421
}
418422
if (candidateGroupingValue == null) {
419423
return SubsumedGroupingsResult.noSubsumption();
@@ -423,7 +427,7 @@ private SubsumedGroupingsResult subsumedGroupings(@Nonnull final Quantifier cand
423427
final BiMap<Value, Value> matchedGroupingsMap;
424428
if (groupingValue != null) {
425429
final var translatedGroupingsValuesBuilder = ImmutableList.<Value>builder();
426-
final var matchedGroupingsMapBuilder = ImmutableBiMap.<Value, Value>builder();
430+
final var matchedGroupingsMapBuilder = ImmutableMap.<Value, Value>builder();
427431
final var groupingValues =
428432
Values.primitiveAccessorsForType(groupingValue.getResultType(), () -> groupingValue).stream()
429433
.map(primitiveGroupingValue -> primitiveGroupingValue.simplify(AliasMap.emptyMap(),
@@ -437,7 +441,14 @@ private SubsumedGroupingsResult subsumedGroupings(@Nonnull final Quantifier cand
437441
matchedGroupingsMapBuilder.put(primitiveGroupingValue, translatedPrimitiveGroupingValue);
438442
}
439443
translatedGroupingValues = translatedGroupingsValuesBuilder.build();
440-
matchedGroupingsMap = matchedGroupingsMapBuilder.build();
444+
445+
//
446+
// We know that if there are duplicates, they will be on the query side. Immutable bi-maps do not support
447+
// duplicated keys at all while regular maps do. The simplest and also the cheapest solution is to just
448+
// use an immutable map builder (which then is de-duped when built) and then use that map to build the
449+
// bi-map.
450+
//
451+
matchedGroupingsMap = ImmutableBiMap.copyOf(matchedGroupingsMapBuilder.buildKeepingLast());
441452
} else {
442453
translatedGroupingValues = ImmutableList.of();
443454
matchedGroupingsMap = ImmutableBiMap.of();
@@ -473,7 +484,7 @@ private SubsumedGroupingsResult subsumedGroupings(@Nonnull final Quantifier cand
473484
// 3. For each candidate grouping value in the set of (yet) unmatched candidate group values, try to find a
474485
// predicate that binds that groupingValue.
475486
//
476-
var booleanWithConstraint = alwaysTrue();
487+
var booleanWithConstraint = BooleanWithConstraint.alwaysTrue();
477488
for (final var translatedGroupingValue : translatedGroupingValuesSet) {
478489
var found = false;
479490

@@ -559,7 +570,8 @@ private SubsumedGroupingsResult subsumedGroupings(@Nonnull final Quantifier cand
559570
}
560571

561572
if (!unmatchedCandidateValues.isEmpty()) {
562-
Verify.verify(candidateGroupingValues.size() > translatedGroupingValues.size());
573+
Verify.verify(candidateGroupingValues.size() > translatedGroupingValuesSet.size());
574+
563575
//
564576
// This is a potential roll-up case, but only if the query side's groupings are completely subsumed
565577
// by the prefix of the candidate side. Iterate up to the smaller query side's grouping values to
@@ -627,14 +639,7 @@ public Compensation compensate(@Nonnull final PartialMatch partialMatch,
627639

628640
final var childCompensation = childCompensationOptional.get();
629641

630-
if (childCompensation.isImpossible()
631-
// ||
632-
// //
633-
// // TODO This needs some improvement as GB a, b, c WHERE a= AND c= needs to reapply the
634-
// // predicate on c which is currently refused here.
635-
// //
636-
// childCompensation.isNeededForFiltering()
637-
) {
642+
if (childCompensation.isImpossible()) {
638643
//
639644
// Note that it may be better to just return the child compensation verbatim as that compensation
640645
// may be combinable with something else to make it possible while the statically impossible compensation
@@ -728,7 +733,7 @@ public static Value flattenedResults(@Nullable final Value groupingKeyValue,
728733
return rcv.simplify(AliasMap.identitiesFor(rcv.getCorrelatedTo()), ImmutableSet.of());
729734
}
730735

731-
public static class UnmatchedAggregateValue extends AbstractValue implements Value.NonEvaluableValue, IndexableAggregateValue {
736+
public static class UnmatchedAggregateValue extends AbstractValue implements Value.NonEvaluableValue {
732737
@Nonnull
733738
private final CorrelationIdentifier unmatchedId;
734739

@@ -747,12 +752,6 @@ protected Iterable<? extends Value> computeChildren() {
747752
return ImmutableList.of();
748753
}
749754

750-
@Nonnull
751-
@Override
752-
public String getIndexTypeName() {
753-
throw new UnsupportedOperationException();
754-
}
755-
756755
@Nonnull
757756
@Override
758757
public ExplainTokensWithPrecedence explain(@Nonnull final Iterable<Supplier<ExplainTokensWithPrecedence>> explainSuppliers) {
@@ -829,7 +828,7 @@ public List<Value> getRollUpToValues() {
829828

830829
@Nonnull
831830
public static SubsumedGroupingsResult noSubsumption() {
832-
return of(falseValue(), ImmutableBiMap.of(), null);
831+
return of(BooleanWithConstraint.falseValue(), ImmutableBiMap.of(), null);
833832
}
834833

835834
@Nonnull

0 commit comments

Comments
 (0)