Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aggregate index support in Cascades #1864

Merged
merged 45 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
447930a
* WIP.
hatyo Sep 13, 2022
3cb497b
* experimentation with matching.
hatyo Sep 20, 2022
1ed9e53
* first version works: matching aggregate index as value index.
hatyo Sep 21, 2022
e487e73
* WIP.
hatyo Sep 22, 2022
8ea4153
* adjustments.
hatyo Sep 22, 2022
7ac45b3
* improve match candidate generation.
hatyo Sep 26, 2022
cf76d8d
* refactoring, more tests.
hatyo Sep 26, 2022
abb70d1
* minor modifications.
hatyo Sep 26, 2022
e9f7ebd
* WIP.
hatyo Sep 29, 2022
b22dc3b
* fixes.
hatyo Sep 30, 2022
39e6e5f
* end to end tests are working.
hatyo Oct 12, 2022
59d45e4
* refactoring + documentation.
hatyo Oct 12, 2022
819fba5
* refactoring + documentation, tests passing again.
hatyo Oct 12, 2022
3ada69f
* further refactoring.
hatyo Oct 12, 2022
a4394bb
* fix rebasing conflicts.
hatyo Oct 12, 2022
79d3031
* clean up.
hatyo Oct 12, 2022
18c792a
* exclude support for count(*) (for now).
hatyo Oct 12, 2022
c23c482
* WIP.
hatyo Oct 13, 2022
a58feea
* tests are passing.
hatyo Oct 14, 2022
189965d
Merge remote-tracking branch 'upstream/main' into aggregate_index_sup…
hatyo Oct 19, 2022
1206df0
* fix indirect merge errors.
hatyo Oct 19, 2022
4ae2fba
* modifications, add ordinal path to FieldValue.
hatyo Oct 25, 2022
d59a608
* add subsumedBy method to Value.
hatyo Oct 25, 2022
b01d6de
* restore broken tests.
hatyo Oct 26, 2022
2e76952
* adapt hash codes.
hatyo Oct 26, 2022
1c0e93f
* address checkstyle violations.
hatyo Oct 26, 2022
b4ea9a3
* address PMD violations.
hatyo Oct 26, 2022
b54097a
* address SonarCloud reported bug.
hatyo Oct 26, 2022
e229b20
* address comments.
hatyo Oct 31, 2022
2f6ad2d
* address comments (2).
hatyo Oct 31, 2022
c96c740
* reference grouping column by ordinal not by name.
hatyo Oct 31, 2022
ba1fae8
Merge remote-tracking branch 'upstream/main' into aggregate_index_sup…
hatyo Oct 31, 2022
d471134
* fix merge skew.
hatyo Oct 31, 2022
17f23d5
* fix checkstyle violations.
hatyo Oct 31, 2022
ba20239
* fix equalsWithoutChildren in AggregateValues.
hatyo Oct 31, 2022
8cb3147
* fix PMD violations.
hatyo Nov 1, 2022
f3e4343
* address comments.
hatyo Nov 3, 2022
bb0c946
* cleanup.
hatyo Nov 3, 2022
5ad81fb
* fix a small bug in MinEverLong instantiation.
hatyo Nov 3, 2022
3ed2994
* address comments (2).
hatyo Nov 3, 2022
849b37a
* factor out Field usage and only rely on FieldPath.
hatyo Nov 7, 2022
4835cea
* fix failing tests.
hatyo Nov 7, 2022
41d8fa3
* code style fixes, review comments.
hatyo Nov 7, 2022
6ec847a
* address more comments.
hatyo Nov 7, 2022
a8081a2
* minor fix in comments.
hatyo Nov 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ The Guava dependency version has been updated to 31.1. Projects may need to chec
* **Feature** Feature 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Feature 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Feature 5 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Support planning aggregate indexes in Cascades. [(Issue #1885)](https://github.com/FoundationDB/fdb-record-layer/issues/1885)
* **Breaking change** Change 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Breaking change** Change 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Breaking change** Change 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
Expand Down Expand Up @@ -690,9 +692,12 @@ public static Map<String, Descriptors.FieldDescriptor> getFieldDescriptorMapFrom

@Nonnull
private static Map<String, Descriptors.FieldDescriptor> getFieldDescriptorMap(@Nonnull final Stream<RecordType> recordTypeStream) {
// todo: should be removed https://github.com/FoundationDB/fdb-record-layer/issues/1884
return recordTypeStream
.sorted(Comparator.comparing(RecordType::getName))
.flatMap(recordType -> recordType.getDescriptor().getFields().stream())
.collect(Collectors.groupingBy(Descriptors.FieldDescriptor::getName,
LinkedHashMap::new,
normen662 marked this conversation as resolved.
Show resolved Hide resolved
normen662 marked this conversation as resolved.
Show resolved Hide resolved
Collectors.reducing(null,
(fieldDescriptor, fieldDescriptor2) -> {
Verify.verify(fieldDescriptor != null || fieldDescriptor2 != null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,12 @@ public String getName() {
return name;
}

/**
* Returns the type of the index.
*
* @return the type of the index.
* @see IndexTypes
*/
@Nonnull
public String getType() {
return type;
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,7 @@ protected Object getBindable() {
}

/**
* Adjust Match Task. Attempts to improve an existing partial partial match on a (group, expression) pair
* Adjust Match Task. Attempts to improve an existing partial match on a (group, expression) pair
normen662 marked this conversation as resolved.
Show resolved Hide resolved
* to a better one by enqueuing rules defined on {@link PartialMatch}.
*
* Simplified enqueue/execute overview:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@
import com.apple.foundationdb.record.metadata.expressions.ThenKeyExpression;
import com.apple.foundationdb.record.query.plan.cascades.KeyExpressionExpansionVisitor.VisitorState;
import com.apple.foundationdb.record.query.plan.cascades.expressions.SelectExpression;
import com.apple.foundationdb.record.query.plan.cascades.predicates.ValueComparisonRangePredicate.Placeholder;
import com.apple.foundationdb.record.query.plan.cascades.values.EmptyValue;
import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue;
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
import com.apple.foundationdb.record.query.plan.cascades.predicates.ValueComparisonRangePredicate.Placeholder;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ default Map<CorrelationIdentifier, ComparisonRange> computeBoundParameterPrefixM
* Compute a list of {@link MatchedOrderingPart}s which forms a bridge to relate {@link KeyExpression}s and
* {@link QueryPredicate}s.
* @param matchInfo a pre-existing match info structure
* @param sortParameterIds the query should be ordered by
* @param sortParameterIds the parameter IDs which the query should be ordered by
* @param isReverse reversed-ness of the order
* @return a list of bound key parts that express the order of the outgoing data stream and their respective mappings
* between query and match candidate
Expand Down Expand Up @@ -207,7 +207,7 @@ default RelationalExpression toEquivalentExpression(@Nonnull final PartialMatch
/**
* Creates a logical expression that represents a scan over the materialized candidate data. This method is expected
* to be implemented by specific implementations of {@link MatchCandidate}.
* @param partialMatch the {@link PartialMatch} that matched th query and the candidate
* @param partialMatch the {@link PartialMatch} that matched the query and the candidate
* @param planContext the plan context for the query
* @param comparisonRanges a {@link List} of {@link ComparisonRange}s to be applied
* @return a new {@link RelationalExpression}
Expand Down Expand Up @@ -237,6 +237,20 @@ default SetMultimap<ExpressionRef<? extends RelationalExpression>, RelationalExp
return refToExpressionMap;
}

@Nonnull
List<RecordType> getQueriedRecordTypes();

int getColumnSize();

boolean isUnique();

@Nonnull
default Set<String> getQueriedRecordTypeNames() {
return getQueriedRecordTypes().stream()
.map(RecordType::getName)
.collect(ImmutableSet.toImmutableSet());
}

@Nonnull
static Iterable<MatchCandidate> fromIndexDefinition(@Nonnull final RecordMetaData metaData,
@Nonnull final Index index,
Expand Down Expand Up @@ -290,6 +304,18 @@ static Iterable<MatchCandidate> fromIndexDefinition(@Nonnull final RecordMetaDat
.ifPresent(resultBuilder::add);
}

if (AggregateIndexExpansionVisitor.isAggregateIndex(type, index.getRootExpression())) {
normen662 marked this conversation as resolved.
Show resolved Hide resolved
expandIndexMatchCandidate(index,
availableRecordTypeNames,
availableRecordTypes,
queriedRecordTypeNames,
queriedRecordTypes,
isReverse,
null,
new AggregateIndexExpansionVisitor(index, queriedRecordTypes))
.ifPresent(resultBuilder::add);
}

return resultBuilder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ public Optional<Boolean> deriveReverseScanOrder() {
}
}

@Nonnull
public MatchInfo withOrderingInfo(@Nonnull final List<MatchedOrderingPart> matchedOrderingParts) {
return new MatchInfo(parameterBindingMap,
quantifierToPartialMatchMap,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public boolean compensationCanBeDeferred() {
}

@Nonnull
public static Collection<MatchInfo> matchesFromMap(@Nonnull IdentityBiMap<Quantifier, PartialMatch> partialMatchMap) {
public static Collection<MatchInfo> matchesFromMap(@Nonnull final IdentityBiMap<Quantifier, PartialMatch> partialMatchMap) {
return partialMatchMap.values()
.stream()
.map(IdentityBiMap::unwrap)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,16 @@ public boolean createsDuplicates() {
return false;
}

@Override
public int getColumnSize() {
return primaryKey.getColumnSize(); // not sure this is correct.
normen662 marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public boolean isUnique() {
return true;
}

@Nonnull
@Override
public RelationalExpression toEquivalentExpression(@Nonnull PartialMatch partialMatch,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@
import java.util.List;

/**
* Visitor that translates a {@link KeyExpression} into a {@link Value}.
* Visitor that translates a {@link KeyExpression} into a {@link Value}, keeping a state of the currently processed
* input type while it is processing the {@link KeyExpression}. The main caveat of this visitor is that it is meant to only
* expand key expressions that are provably scalar.
*/
@SuppressWarnings("java:S5993")
public class ScalarTranslationVisitor implements KeyExpressionVisitor<ScalarTranslationVisitor.ScalarVisitorState, Value> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

package com.apple.foundationdb.record.query.plan.cascades;

import com.apple.foundationdb.record.metadata.Index;
import com.apple.foundationdb.record.query.plan.cascades.values.Value;

import javax.annotation.Nonnull;
Expand All @@ -31,9 +30,6 @@
*/
public interface ScanWithFetchMatchCandidate extends WithPrimaryKeyMatchCandidate {

@Nonnull
Index getIndex();

@Nonnull
Optional<Value> pushValueThroughFetch(@Nonnull Value value,
@Nonnull CorrelationIdentifier sourceAlias,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,14 @@ public ValueIndexScanMatchCandidate(@Nonnull Index index,
this.primaryKeyValuesSupplier = Suppliers.memoize(() -> MatchCandidate.computePrimaryKeyValuesMaybe(primaryKey, baseType));
}

@Nonnull
@Override
public Index getIndex() {
return index;
public int getColumnSize() {
return index.getColumnSize();
}

@Override
public boolean isUnique() {
return index.isUnique();
}

@Nonnull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,14 @@ public WindowedIndexScanMatchCandidate(@Nonnull Index index,
this.primaryKeyValuesSupplier = Suppliers.memoize(() -> MatchCandidate.computePrimaryKeyValuesMaybe(primaryKey, baseType));
}

@Nonnull
@Override
public Index getIndex() {
return index;
public int getColumnSize() {
return index.getColumnSize();
}

@Override
public boolean isUnique() {
return index.isUnique();
}

@Nonnull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,11 @@

package com.apple.foundationdb.record.query.plan.cascades;

import com.apple.foundationdb.record.metadata.RecordType;
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
import com.google.common.collect.ImmutableSet;

import javax.annotation.Nonnull;
import java.util.List;
import java.util.Optional;
import java.util.Set;

/**
* Interface to represent a candidate that uses a primary key to identify a record.
Expand All @@ -36,16 +33,6 @@ public interface WithPrimaryKeyMatchCandidate extends MatchCandidate {
@Nonnull
Optional<List<Value>> getPrimaryKeyValuesMaybe();

@Nonnull
normen662 marked this conversation as resolved.
Show resolved Hide resolved
List<RecordType> getQueriedRecordTypes();

@Nonnull
default Set<String> getQueriedRecordTypeNames() {
return getQueriedRecordTypes().stream()
.map(RecordType::getName)
.collect(ImmutableSet.toImmutableSet());
}

@Nonnull
static Optional<List<Value>> commonPrimaryKeyValuesMaybe(@Nonnull Iterable<? extends MatchCandidate> matchCandidates) {
List<Value> common = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ public class NodeInfo {
NodeIcon.DATA_ACCESS_OPERATOR,
"Covering Index Scan",
"A covering index scan operator uses a secondary index to quickly find records in a given search range without looking up the base records.");

public static final NodeInfo AGGREGATE_INDEX_SCAN_OPERATOR = new NodeInfo(
normen662 marked this conversation as resolved.
Show resolved Hide resolved
"AggregateIndexScanOperator",
NodeIcon.DATA_ACCESS_OPERATOR,
"Aggregate Index Scan",
"An aggregate index scan operator uses a secondary index to quickly find records in a given search range without looking up the base records.");

public static final NodeInfo COVERING_SPATIAL_INDEX_SCAN_OPERATOR = new NodeInfo(
"CoveringSpatialIndexScanOperator",
NodeIcon.DATA_ACCESS_OPERATOR,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,14 @@
import com.apple.foundationdb.annotation.API;
import com.apple.foundationdb.record.query.plan.cascades.AliasMap;
import com.apple.foundationdb.record.query.plan.cascades.Column;
import com.apple.foundationdb.record.query.plan.cascades.ComparisonRange;
import com.apple.foundationdb.record.query.plan.cascades.Compensation;
import com.apple.foundationdb.record.query.plan.cascades.CorrelationIdentifier;
import com.apple.foundationdb.record.query.plan.cascades.IdentityBiMap;
import com.apple.foundationdb.record.query.plan.cascades.OrderingPart;
import com.apple.foundationdb.record.query.plan.cascades.MatchInfo;
import com.apple.foundationdb.record.query.plan.cascades.PartialMatch;
import com.apple.foundationdb.record.query.plan.cascades.PredicateMap;
import com.apple.foundationdb.record.query.plan.cascades.Quantifier;
import com.apple.foundationdb.record.query.plan.cascades.RequestedOrdering;
import com.apple.foundationdb.record.query.plan.cascades.TranslationMap;
Expand All @@ -46,6 +52,7 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -249,6 +256,42 @@ public Value getRuntimeValue() {
return computeRuntimeResultSupplier.get();
}

@Override
public Compensation compensate(@Nonnull final PartialMatch partialMatch,
@Nonnull final Map<CorrelationIdentifier, ComparisonRange> boundParameterPrefixMap) {
// subsumedBy() is based on equality, thus we return empty here as
// if there is a match, it's exact
return Compensation.noCompensation();
}

@Nonnull
@Override
public Iterable<MatchInfo> subsumedBy(@Nonnull final RelationalExpression candidateExpression,
@Nonnull final AliasMap aliasMap,
@Nonnull final IdentityBiMap<Quantifier, PartialMatch> partialMatchMap) {

// the candidate must be a GROUP-BY expression.
if (candidateExpression.getClass() != this.getClass()) {
return ImmutableList.of();
}

final var otherGroupByExpression = (GroupByExpression)candidateExpression;

// the grouping values are encoded directly in the underlying SELECT-WHERE, reaching this point means that the
// grouping values had exact match so we don't need to check them.


// check that aggregate value is the same.
final var otherAggregateValue = otherGroupByExpression.getAggregateValue();
if (aggregateValue.subsumedBy(otherAggregateValue, aliasMap)) {
// placeholder for information needed for later compensation.
return MatchInfo.tryMerge(partialMatchMap, ImmutableMap.of(), PredicateMap.empty(), Optional.empty())
.map(ImmutableList::of)
.orElse(ImmutableList.of());
}
return ImmutableList.of();
}

@Nonnull
private Value computeResultValue() {
final var aggregateColumn = Column.of(Type.Record.Field.of(getAggregateValue().getResultType(), Optional.of(getAggregateValueAlias().getId())), getAggregateValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public PartiallyOrderedSet<CorrelationIdentifier> getCorrelationOrder() {
private PartiallyOrderedSet<CorrelationIdentifier> computeCorrelationOrder() {
return RelationalExpressionWithChildren.ChildrenAsSet.super.getCorrelationOrder();
}

@Nonnull
@Override
@SuppressWarnings("PMD.CompareObjectsWithEquals")
Expand Down Expand Up @@ -360,16 +360,16 @@ public Iterable<MatchInfo> subsumedBy(@Nonnull final RelationalExpression candid
// FROM R r, S s
// WHERE r.a < 5 AND s.b = 10 AND r.x = s.y
//
// The predicate 'r.x = r.y' can be used as a predicate for matching an index on R(x, a) or for
// The predicate 'r.x = s.y' can be used as a predicate for matching an index on R(x, a) or for
// matching an index on S(b, y). In fact the predicate needs to be shared in some way such that the planner
// can later on make the right decision based on cost, etc.
//
// The way this is implemented is to create two matches one where the predicate is repossessed to the match
// at hand. When we match R(x, a) we repossess r.x = r.y to be subsumed by r.x = ? on
// the candidate side. Vica versa, when we match S(b, y) we repossess s.y = r.x to be subsumed by s.y = ? on the
// the candidate side. Vice versa, when we match S(b, y) we repossess s.y = r.x to be subsumed by s.y = ? on the
// candidate side.
//
// Using this approach we create a problem that these two matches cannot coexist in a way that they cannot
// Using this approach we create a problem that these two matches can coexist in a way that they cannot
// be realized, that is planned together at all as both matches provide the other's placeholder value. In fact,
// we have forced the match to (if it were to be planned) become the inner of a join. It would be beneficial,
// however, to also create a version of the match that does not consider the join predicate at all.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.apple.foundationdb.record.query.plan.cascades.expressions.RelationalExpressionWithPredicates;
import com.apple.foundationdb.record.query.plan.cascades.expressions.SelectExpression;
import com.apple.foundationdb.record.query.plan.cascades.predicates.QueryPredicate;
import com.apple.foundationdb.record.query.plan.cascades.values.RecordConstructorValue;
import com.apple.foundationdb.record.query.plan.plans.RecordQueryPlan;
import com.google.common.collect.ImmutableList;

Expand Down Expand Up @@ -234,4 +235,19 @@ public static BindingMatcher<ExplodeExpression> explodeExpression() {
public static BindingMatcher<GroupByExpression> groupByExpression(@Nonnull final CollectionMatcher<? extends Quantifier> downstream) {
return ofTypeOwning(GroupByExpression.class, downstream);
}

@Nonnull
public static BindingMatcher<GroupByExpression> groupByExpression(@Nonnull final BindingMatcher<? extends RecordConstructorValue> downstreamAggregation,
@Nonnull final CollectionMatcher<? extends Quantifier> downstreamQuantifiers) {
return typedWithDownstream(GroupByExpression.class,
Extractor.identity(),
AllOfMatcher.matchingAllOf(GroupByExpression.class,
ImmutableList.of(
typedWithDownstream(GroupByExpression.class,
Extractor.of(GroupByExpression::getAggregateValue, name -> "aggregation(" + name + ")"),
downstreamAggregation),
typedWithDownstream(GroupByExpression.class,
Extractor.of(RelationalExpression::getQuantifiers, name -> "quantifiers(" + name + ")"),
downstreamQuantifiers))));
}
}