From 8a0596e1ae6a89e6cd9c3d1a7082b366160b3674 Mon Sep 17 00:00:00 2001 From: Youssef Hatem Date: Fri, 20 Mar 2026 19:33:57 +0000 Subject: [PATCH] checkpoint, scan from multi-type primary scan test works. --- .../expressions/RecordTypeKeyComparison.java | 2 +- .../AggregateIndexExpansionVisitor.java | 10 +- .../query/plan/cascades/ExpansionVisitor.java | 11 +- .../query/plan/cascades/GraphExpansion.java | 22 +++ .../cascades/MatchCandidateExpansion.java | 27 ++-- .../PrimaryAccessExpansionVisitor.java | 39 +++++- .../cascades/ValueIndexExpansionVisitor.java | 9 +- .../cascades/VectorIndexExpansionVisitor.java | 9 +- .../WindowedIndexExpansionVisitor.java | 12 +- .../LogicalTypeFilterExpression.java | 130 +++++++++++++++++- .../expressions/RelationalExpression.java | 2 +- .../plan/cascades/predicates/Placeholder.java | 8 ++ .../query/FDBModificationQueryTest.java | 12 +- .../query/FDBQueryGraphTestHelpers.java | 2 +- .../foundationdb/query/GroupByTest.java | 4 +- .../query/RecursiveQueriesTest.java | 2 +- .../foundationdb/query/SparseIndexTest.java | 2 +- .../query/plan/cascades/RuleTestHelper.java | 4 +- .../recordlayer/query/LogicalOperator.java | 2 +- .../src/test/java/YamlIntegrationTests.java | 7 + .../record-type-key-tests.metrics.binpb | 15 ++ .../record-type-key-tests.metrics.yaml | 12 ++ .../resources/record-type-key-tests.yamsql | 36 +++++ 23 files changed, 325 insertions(+), 54 deletions(-) create mode 100644 yaml-tests/src/test/resources/record-type-key-tests.metrics.binpb create mode 100644 yaml-tests/src/test/resources/record-type-key-tests.metrics.yaml create mode 100644 yaml-tests/src/test/resources/record-type-key-tests.yamsql diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/expressions/RecordTypeKeyComparison.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/expressions/RecordTypeKeyComparison.java index 9c25f3d425..c224e62b3f 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/expressions/RecordTypeKeyComparison.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/expressions/RecordTypeKeyComparison.java @@ -172,7 +172,7 @@ public static class RecordTypeComparison implements Comparisons.Comparison { @Nonnull private final String recordTypeName; - RecordTypeComparison(@Nonnull String recordTypeName) { + public RecordTypeComparison(@Nonnull String recordTypeName) { this.recordTypeName = recordTypeName; } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AggregateIndexExpansionVisitor.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AggregateIndexExpansionVisitor.java index b2a7bf3345..2595b41004 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AggregateIndexExpansionVisitor.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AggregateIndexExpansionVisitor.java @@ -64,6 +64,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Stream; @@ -119,17 +120,18 @@ public boolean isPermuted() { * * @param baseQuantifierSupplier a quantifier supplier to create base data access * @param ignored the primary key of the data object the caller wants to access, this parameter is ignored since - * an aggregate index does not possess primary key information, must be {@code null}. + * an aggregate index does not possess primary key information, must be {@code null}. * @param isReverse an indicator whether the result set is expected to be returned in reverse order. + * * @return A match candidate representing the aggregate index. */ @Nonnull @Override - public MatchCandidate expand(@Nonnull final Supplier baseQuantifierSupplier, + public MatchCandidate expand(@Nonnull final Function, Quantifier.ForEach> baseQuantifierSupplier, @Nullable final KeyExpression ignored, final boolean isReverse) { Verify.verify(ignored == null); - final var baseQuantifier = baseQuantifierSupplier.get(); + final var baseQuantifier = baseQuantifierSupplier.apply(Optional.empty()); // todo // 0. create a base expansion to resolve the key expression to columns with appropriate quantifiers final var baseExpansion = constructBaseExpansion(baseQuantifier); @@ -150,6 +152,8 @@ public MatchCandidate expand(@Nonnull final Supplier baseQua // 3. construct SELECT-HAVING with SORT on top. final var constructSelectHavingResult = constructSelectHaving(groupByQun, placeholders); final var selectHaving = constructSelectHavingResult.getSelectExpression(); + + // todo, prepend the additional parameters. final var placeHolderAliases = constructSelectHavingResult.getPlaceholderAliases(); // 4. add sort on top, if necessary, this will be absorbed later on as an ordering property of the match candidate. diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/ExpansionVisitor.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/ExpansionVisitor.java index ed9131c320..a35ee90514 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/ExpansionVisitor.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/ExpansionVisitor.java @@ -24,7 +24,8 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.util.function.Supplier; +import java.util.Optional; +import java.util.function.Function; /** * A sub interface of {@link KeyExpressionVisitor} that fixes the return type to be a {@link GraphExpansion} and @@ -36,13 +37,15 @@ public interface ExpansionVisitor extends KeyExpressionVisitor { /** * Method that expands a data structure into a data flow graph. + * * @param baseQuantifierSupplier a quantifier supplier to create base data access * @param primaryKey the primary key of the data object the caller wants to access * @param isReverse an indicator whether the result set is expected to be returned in reverse order + * * @return a new {@link MatchCandidate} that can be used for matching. */ @Nonnull - MatchCandidate expand(@Nonnull Supplier baseQuantifierSupplier, - @Nullable KeyExpression primaryKey, - boolean isReverse); + MatchCandidate expand(@Nonnull final Function, Quantifier.ForEach> baseQuantifierSupplier, + @Nullable final KeyExpression primaryKey, + final boolean isReverse); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/GraphExpansion.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/GraphExpansion.java index 08ac3ac475..db5b09befa 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/GraphExpansion.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/GraphExpansion.java @@ -27,6 +27,7 @@ import com.apple.foundationdb.record.query.plan.cascades.predicates.PredicateWithValueAndRanges; 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.cascades.values.RecordTypeValue; import com.apple.foundationdb.record.query.plan.cascades.values.Value; import com.apple.foundationdb.record.util.pair.NonnullPair; import com.google.common.base.Verify; @@ -39,6 +40,7 @@ import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -512,6 +514,26 @@ public Builder addPredicate(@Nonnull final QueryPredicate predicate) { return this; } + @Nonnull + public Builder replacePlaceholder(@Nonnull final Function replacementFunction) { + final ImmutableList currentPlaceholders = placeholders.build(); + placeholders = new ImmutableList.Builder<>(); + for (final Placeholder placeholder : currentPlaceholders) { + placeholders.add(replacementFunction.apply(placeholder)); + } + return this; + } + + @Nonnull + public Builder replacePredicate(@Nonnull final Function replacementFunction) { + final ImmutableList currentPlaceholders = predicates.build(); + predicates = new ImmutableList.Builder<>(); + for (final QueryPredicate predicate : currentPlaceholders) { + predicates.add(replacementFunction.apply(predicate)); + } + return this; + } + @Nonnull public Builder addAllPredicates(@Nonnull final Iterable addPredicates) { predicates.addAll(addPredicates); diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/MatchCandidateExpansion.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/MatchCandidateExpansion.java index b07f3e1cf3..750f9b5be2 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/MatchCandidateExpansion.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/MatchCandidateExpansion.java @@ -99,9 +99,9 @@ public static Optional expandAggregateIndexMatchCandidate(@Nonnu public static Optional expandIndexMatchCandidate(@Nonnull IndexExpansionInfo info, @Nullable KeyExpression commonPrimaryKey, @Nonnull final ExpansionVisitor expansionVisitor) { - final var baseRef = createBaseRef(info, new IndexAccessHint(info.getIndexName())); try { - return Optional.of(expansionVisitor.expand(() -> Quantifier.forEach(baseRef), commonPrimaryKey, info.isReverse())); + return Optional.of(expansionVisitor.expand(aliasMaybe -> Quantifier.forEach(createBaseRef(info, aliasMaybe, + new IndexAccessHint(info.getIndexName()))), commonPrimaryKey, info.isReverse())); } catch (final UnsupportedOperationException uOE) { // just log and return empty if (LOGGER.isDebugEnabled()) { @@ -127,24 +127,30 @@ public static Optional fromPrimaryDefinition(@Nonnull final Reco .filter(recordType -> queriedRecordTypeNames.contains(recordType.getName())) .collect(ImmutableList.toImmutableList()); - final var baseRef = createBaseRef(metaData.getRecordTypes().keySet(), queriedRecordTypeNames, metaData.getPlannerType(queriedRecordTypeNames), new PrimaryAccessHint()); final var expansionVisitor = new PrimaryAccessExpansionVisitor(availableRecordTypes, queriedRecordTypes); - return Optional.of(expansionVisitor.expand(() -> Quantifier.forEach(baseRef), primaryKey, isReverse)); + // I don't think we need to put the parameters here, but let's see if that's necessary. + return Optional.of(expansionVisitor.expand(aliasMaybe -> + Quantifier.forEach(createBaseRef(metaData.getRecordTypes().keySet(), + queriedRecordTypeNames, metaData.getPlannerType(queriedRecordTypeNames), aliasMaybe, new PrimaryAccessHint())), primaryKey, isReverse)); } return Optional.empty(); } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") @Nonnull private static Reference createBaseRef(@Nonnull IndexExpansionInfo info, - @Nonnull AccessHint accessHint) { - return createBaseRef(info.getAvailableRecordTypeNames(), info.getIndexedRecordTypeNames(), info.getBaseType(), accessHint); + @Nonnull final Optional recordTypeKeyAliasMaybe, + @Nonnull AccessHint accessHint) { + return createBaseRef(info.getAvailableRecordTypeNames(), info.getIndexedRecordTypeNames(), info.getBaseType(), recordTypeKeyAliasMaybe, accessHint); } @Nonnull + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") private static Reference createBaseRef(@Nonnull final Set availableRecordTypeNames, @Nonnull final Set queriedRecordTypeNames, @Nonnull final Type.Record baseType, + @Nonnull final Optional recordTypeKeyAliasMaybe, @Nonnull AccessHint accessHint) { final var quantifier = Quantifier.forEach( @@ -152,9 +158,10 @@ private static Reference createBaseRef(@Nonnull final Set availableRecor new FullUnorderedScanExpression(availableRecordTypeNames, new Type.AnyRecord(false), new AccessHints(accessHint)))); - return Reference.initialOf( - new LogicalTypeFilterExpression(queriedRecordTypeNames, - quantifier, - baseType)); + return Reference.initialOf(LogicalTypeFilterExpression.newInstanceForMatchCandidate(queriedRecordTypeNames, + availableRecordTypeNames, + quantifier, + recordTypeKeyAliasMaybe, + baseType)); } } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PrimaryAccessExpansionVisitor.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PrimaryAccessExpansionVisitor.java index e67058d235..90f54ef013 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PrimaryAccessExpansionVisitor.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PrimaryAccessExpansionVisitor.java @@ -22,11 +22,15 @@ import com.apple.foundationdb.annotation.SpotBugsSuppressWarnings; import com.apple.foundationdb.record.RecordCoreException; +import com.apple.foundationdb.record.metadata.Key; import com.apple.foundationdb.record.metadata.RecordType; import com.apple.foundationdb.record.metadata.expressions.KeyExpression; import com.apple.foundationdb.record.query.plan.cascades.debug.Debugger; import com.apple.foundationdb.record.query.plan.cascades.expressions.MatchableSortExpression; +import com.apple.foundationdb.record.query.plan.cascades.predicates.Placeholder; import com.apple.foundationdb.record.query.plan.cascades.predicates.PredicateWithValueAndRanges; +import com.apple.foundationdb.record.query.plan.cascades.values.RecordTypeValue; +import com.apple.foundationdb.record.util.pair.NonnullPair; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; @@ -35,11 +39,13 @@ import java.util.Collection; import java.util.List; import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; import java.util.function.Supplier; /** * Class to expand primary data access into a candidate. The visitation methods are left unchanged from the super class - * {@link KeyExpressionExpansionVisitor}, this class merely provides a specific {@link #expand} method. + * {@link KeyExpressionExpansionVisitor}, this class merely provides a specific {@link ExpansionVisitor#expand} method. */ public class PrimaryAccessExpansionVisitor extends KeyExpressionExpansionVisitor implements ExpansionVisitor { @Nonnull @@ -55,16 +61,22 @@ public PrimaryAccessExpansionVisitor(@Nonnull final Collection avail @Nonnull @Override @SpotBugsSuppressWarnings("NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE") - public PrimaryScanMatchCandidate expand(@Nonnull final Supplier baseQuantifierSupplier, + public PrimaryScanMatchCandidate expand(@Nonnull final Function, Quantifier.ForEach> baseQuantifierSupplier, @Nullable final KeyExpression primaryKey, final boolean isReverse) { Objects.requireNonNull(primaryKey); Debugger.updateIndex(PredicateWithValueAndRanges.class, old -> 0); - final var baseQuantifier = baseQuantifierSupplier.get(); + final Optional recordTypeQuantifierMaybe; + if (Key.Expressions.recordType().isPrefixKey(primaryKey)) { + recordTypeQuantifierMaybe = Optional.of(newParameterAlias()); + } else { + recordTypeQuantifierMaybe = Optional.empty(); + } + final Quantifier.ForEach baseQuantifier = baseQuantifierSupplier.apply(recordTypeQuantifierMaybe); // expand - final var graphExpansion = + final var graphExpansionBuilder = pop(primaryKey.expand(push(VisitorState.of(Lists.newArrayList(), Lists.newArrayList(), baseQuantifier, @@ -74,8 +86,23 @@ public PrimaryScanMatchCandidate expand(@Nonnull final Supplier graphExpansionBuilder.replacePlaceholder(placeholder -> { + if (placeholder.getValue() instanceof RecordTypeValue) { + return placeholder.withAlias(correlationIdentifier); + } + return placeholder; + })); + + recordTypeQuantifierMaybe.ifPresent(correlationIdentifier -> graphExpansionBuilder.replacePredicate(placeholder -> { + if (placeholder instanceof Placeholder && ((Placeholder)placeholder).getValue() instanceof RecordTypeValue) { + return ((Placeholder)placeholder).withAlias(correlationIdentifier); + } + return placeholder; + })); + + final var graphExpansion = graphExpansionBuilder.build(); final var allExpansions = GraphExpansion.ofOthers(GraphExpansion.ofQuantifier(baseQuantifier), graphExpansion); diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/ValueIndexExpansionVisitor.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/ValueIndexExpansionVisitor.java index 9469553b7d..1c83ff5669 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/ValueIndexExpansionVisitor.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/ValueIndexExpansionVisitor.java @@ -43,14 +43,15 @@ import java.util.Collection; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; -import java.util.function.Supplier; +import java.util.function.Function; import static com.apple.foundationdb.record.metadata.Key.Expressions.concat; /** * Class to expand value index access into a candidate graph. The visitation methods are left unchanged from the super - * class {@link KeyExpressionExpansionVisitor}, this class merely provides a specific {@link #expand} method. + * class {@link KeyExpressionExpansionVisitor}, this class merely provides a specific {@link ExpansionVisitor#expand} method. */ public class ValueIndexExpansionVisitor extends KeyExpressionExpansionVisitor implements ExpansionVisitor { // We may need to rethink this as it limits the set of indexes that can support a grouping key expression @@ -76,12 +77,12 @@ public ValueIndexExpansionVisitor(@Nonnull Index index, @Nonnull Collection baseQuantifierSupplier, + public MatchCandidate expand(@Nonnull final Function, Quantifier.ForEach> baseQuantifierSupplier, @Nullable final KeyExpression primaryKey, final boolean isReverse) { Debugger.updateIndex(PredicateWithValueAndRanges.class, old -> 0); - final var baseQuantifier = baseQuantifierSupplier.get(); + final var baseQuantifier = baseQuantifierSupplier.apply(Optional.empty()); // todo final var allExpansionsBuilder = ImmutableList.builder(); // add the value for the flow of records diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/VectorIndexExpansionVisitor.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/VectorIndexExpansionVisitor.java index e35a471533..5160fe2b0b 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/VectorIndexExpansionVisitor.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/VectorIndexExpansionVisitor.java @@ -40,6 +40,7 @@ import com.apple.foundationdb.record.query.plan.cascades.values.EuclideanDistanceRowNumberValue; import com.apple.foundationdb.record.query.plan.cascades.values.EuclideanSquareDistanceRowNumberValue; import com.apple.foundationdb.record.query.plan.cascades.values.Value; +import com.apple.foundationdb.record.util.pair.NonnullPair; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -50,12 +51,14 @@ import java.util.Collection; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; +import java.util.function.Function; import java.util.function.Supplier; /** * Class to expand vector index access into a candidate graph. The visitation methods are left unchanged from the super - * class {@link KeyExpressionExpansionVisitor}, this class merely provides a specific {@link #expand} method. + * class {@link KeyExpressionExpansionVisitor}, this class merely provides a specific {@link ExpansionVisitor#expand} method. */ public class VectorIndexExpansionVisitor extends KeyExpressionExpansionVisitor implements ExpansionVisitor { @Nonnull @@ -77,12 +80,12 @@ public VectorIndexExpansionVisitor(@Nonnull Index index, @Nonnull Collection baseQuantifierSupplier, + public MatchCandidate expand(@Nonnull final Function, Quantifier.ForEach> baseQuantifierSupplier, @Nullable final KeyExpression primaryKey, final boolean isReverse) { Debugger.updateIndex(PredicateWithValueAndRanges.class, old -> 0); - final var baseQuantifier = baseQuantifierSupplier.get(); + final var baseQuantifier = baseQuantifierSupplier.apply(Optional.empty()); // todo final var allExpansionsBuilder = ImmutableList.builder(); allExpansionsBuilder.add(GraphExpansion.ofQuantifier(baseQuantifier)); diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/WindowedIndexExpansionVisitor.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/WindowedIndexExpansionVisitor.java index 8cdfee3036..f822160591 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/WindowedIndexExpansionVisitor.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/WindowedIndexExpansionVisitor.java @@ -35,6 +35,7 @@ import com.apple.foundationdb.record.query.plan.cascades.values.QuantifiedObjectValue; import com.apple.foundationdb.record.query.plan.cascades.values.RankValue; import com.apple.foundationdb.record.query.plan.cascades.values.Value; +import com.apple.foundationdb.record.util.pair.NonnullPair; import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; @@ -46,11 +47,13 @@ import javax.annotation.Nullable; import java.util.Collection; import java.util.List; +import java.util.Optional; +import java.util.function.Function; import java.util.function.Supplier; /** * Class to expand a by-rank index access into a candidate graph. The visitation methods are left unchanged from the super - * class {@link KeyExpressionExpansionVisitor}, this class merely provides a specific {@link #expand} method. + * class {@link KeyExpressionExpansionVisitor}, this class merely provides a specific {@link ExpansionVisitor#expand} method. */ public class WindowedIndexExpansionVisitor extends KeyExpressionExpansionVisitor implements ExpansionVisitor { @Nonnull @@ -89,11 +92,12 @@ public WindowedIndexExpansionVisitor(@Nonnull Index index, @Nonnull Collection baseQuantifierSupplier, + public MatchCandidate expand(@Nonnull final Function, Quantifier.ForEach> baseQuantifierSupplier, @Nullable final KeyExpression primaryKey, final boolean isReverse) { var rootExpression = index.getRootExpression(); @@ -102,7 +106,7 @@ public MatchCandidate expand(@Nonnull final Supplier baseQua Debugger.updateIndex(PredicateWithValueAndRanges.class, old -> 0); final var allExpansionsBuilder = ImmutableList.builder(); - final var baseQuantifier = baseQuantifierSupplier.get(); + final var baseQuantifier = baseQuantifierSupplier.apply(Optional.empty()); // todo final var baseExpansion = GraphExpansion.builder() @@ -119,7 +123,7 @@ public MatchCandidate expand(@Nonnull final Supplier baseQua // TODO verify if there is only ever going to be a grouped count of 1, for now assert on it Verify.verify(groupingKeyExpression.getGroupedCount() == 1); - final var innerBaseQuantifier = baseQuantifierSupplier.get(); + final var innerBaseQuantifier = baseQuantifierSupplier.apply(Optional.empty()); // todo (refactor as well) final var innerBaseAlias = innerBaseQuantifier.getAlias(); final var expandGroupingsAndArgumentsResult = expandGroupingsAndArguments(baseQuantifier, innerBaseQuantifier, groupingKeyExpression, diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/LogicalTypeFilterExpression.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/LogicalTypeFilterExpression.java index c351c20b40..e8b048ca38 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/LogicalTypeFilterExpression.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/LogicalTypeFilterExpression.java @@ -23,36 +23,53 @@ import com.apple.foundationdb.annotation.API; import com.apple.foundationdb.record.EvaluationContext; import com.apple.foundationdb.record.RecordCoreException; +import com.apple.foundationdb.record.query.expressions.Comparisons; +import com.apple.foundationdb.record.query.expressions.RecordTypeKeyComparison; import com.apple.foundationdb.record.query.plan.cascades.AliasMap; 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.GroupByMappings; import com.apple.foundationdb.record.query.plan.cascades.IdentityBiMap; import com.apple.foundationdb.record.query.plan.cascades.LinkedIdentityMap; 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.PredicateMultiMap; import com.apple.foundationdb.record.query.plan.cascades.Quantifier; +import com.apple.foundationdb.record.query.plan.cascades.Quantifiers; +import com.apple.foundationdb.record.query.plan.cascades.ValueEquivalence; import com.apple.foundationdb.record.query.plan.cascades.explain.Attribute; import com.apple.foundationdb.record.query.plan.cascades.explain.NodeInfo; import com.apple.foundationdb.record.query.plan.cascades.explain.PlannerGraph; import com.apple.foundationdb.record.query.plan.cascades.explain.PlannerGraphRewritable; +import com.apple.foundationdb.record.query.plan.cascades.predicates.Placeholder; +import com.apple.foundationdb.record.query.plan.cascades.predicates.QueryPredicate; +import com.apple.foundationdb.record.query.plan.cascades.predicates.ValuePredicate; import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.apple.foundationdb.record.query.plan.cascades.values.QuantifiedObjectValue; +import com.apple.foundationdb.record.query.plan.cascades.values.QuantifiedRecordValue; +import com.apple.foundationdb.record.query.plan.cascades.values.RecordTypeValue; import com.apple.foundationdb.record.query.plan.cascades.values.Value; +import com.apple.foundationdb.record.query.plan.cascades.values.translation.MaxMatchMap; import com.apple.foundationdb.record.query.plan.cascades.values.translation.PullUp; import com.apple.foundationdb.record.query.plan.cascades.values.translation.TranslationMap; +import com.apple.foundationdb.record.util.pair.NonnullPair; +import com.google.common.base.Predicate; import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Streams; 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; +import java.util.stream.Collectors; /** * A relational planner expression that represents an unimplemented type filter on the records produced by its inner @@ -67,11 +84,15 @@ public class LogicalTypeFilterExpression extends AbstractRelationalExpressionWit private final Quantifier innerQuantifier; @Nonnull private final Type resultType; + @Nonnull + private final List predicates; - public LogicalTypeFilterExpression(@Nonnull Set recordTypes, @Nonnull Quantifier innerQuantifier, @Nonnull Type resultType) { + private LogicalTypeFilterExpression(@Nonnull final Set recordTypes, @Nonnull final Quantifier innerQuantifier, + @Nonnull final Type resultType, @Nonnull final List predicates) { this.recordTypes = recordTypes; this.innerQuantifier = innerQuantifier; this.resultType = resultType; + this.predicates = ImmutableList.copyOf(predicates); } @Nonnull @@ -113,8 +134,12 @@ public Set computeCorrelatedToWithoutChildren() { public LogicalTypeFilterExpression translateCorrelations(@Nonnull final TranslationMap translationMap, final boolean shouldSimplifyValues, @Nonnull final List translatedQuantifiers) { + final var translatedPredicates = + predicates.stream() + .map(p -> p.translateCorrelations(translationMap, shouldSimplifyValues)) + .collect(Collectors.toList()); return new LogicalTypeFilterExpression(getRecordTypes(), Iterables.getOnlyElement(translatedQuantifiers), - resultType); + resultType, translatedPredicates); } @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") @@ -123,6 +148,30 @@ public boolean equals(final Object other) { return semanticEquals(other); } + @Override + public boolean equalsWithoutChildren(@Nonnull final RelationalExpression otherExpression, @Nonnull final AliasMap equivalencesMap) { + if (this == otherExpression) { + return true; + } + + if (getClass() != otherExpression.getClass()) { + return false; + } + + final var otherLogicalTypeFilterExpression = (LogicalTypeFilterExpression)otherExpression; + if (!getRecordTypes().equals(otherLogicalTypeFilterExpression.getRecordTypes())) { + return false; + } + + final var otherPredicates = otherLogicalTypeFilterExpression.getPredicates(); + return TypeFilterExpression.super.equalsWithoutChildren(otherExpression, equivalencesMap) && + predicates.size() == otherPredicates.size() && + Streams.zip(predicates.stream(), + otherPredicates.stream(), + (queryPredicate, otherQueryPredicate) -> queryPredicate.semanticEquals(otherQueryPredicate, equivalencesMap)) + .allMatch(isSame -> isSame); + } + @Override public int hashCode() { return semanticHashCode(); @@ -130,7 +179,11 @@ public int hashCode() { @Override public int computeHashCodeWithoutChildren() { - return TypeFilterExpression.super.computeHashCodeWithoutChildren(); + if (predicates.isEmpty()) { + return TypeFilterExpression.super.computeHashCodeWithoutChildren(); + } else { + return Objects.hash(getRecordTypes(), predicates); + } } @Nonnull @@ -175,7 +228,46 @@ public Iterable subsumedBy(@Nonnull final RelationalExpression candid return ImmutableList.of(); } - return exactlySubsumedBy(candidateExpression, bindingAliasMap, partialMatchMap, translationMapOptional.get()); + final var translatedResultValue = + getResultValue().translateCorrelations(translationMapOptional.get(), true); + + if (candidateTypeFilterExpression.getPredicates().isEmpty()) { + return exactlySubsumedBy(candidateExpression, bindingAliasMap, partialMatchMap, translationMapOptional.get()); + } else { + if (recordTypes.size() > 1 || candidateTypeFilterExpression.getPredicates().size() != 1) { + // we can't plan queries across multiple types now. + return ImmutableList.of(); + } + final String recordTypeName = Iterables.getOnlyElement(recordTypes); + final QueryPredicate candidatePredicate = Iterables.getOnlyElement(candidateTypeFilterExpression.getPredicates()); + final Comparisons.Comparison comparison = new RecordTypeKeyComparison.RecordTypeComparison(recordTypeName); + final var translationMap = translationMapOptional.get(); + final QueryPredicate queryPredicate = new ValuePredicate(new RecordTypeValue(QuantifiedRecordValue.of(innerQuantifier)), comparison); + final QueryPredicate translatedPredicate = queryPredicate.translateCorrelations(translationMap, true); + final var placeholderAlias = ((Placeholder)candidatePredicate).getParameterAlias(); + + final var mapping = PredicateMultiMap.PredicateMapping.regularMappingBuilder(queryPredicate, translatedPredicate, candidatePredicate) + .setParameterAlias(placeholderAlias).build(); + + final var predicateMappingBuilder = PredicateMultiMap.builder(); + predicateMappingBuilder.put(queryPredicate, mapping); + final var predicateMappingMaybe = predicateMappingBuilder.buildMaybe(); + + if (predicateMappingMaybe.isEmpty()) { + return ImmutableList.of(); + } + + ComparisonRange comparisonRange = ComparisonRange.from(comparison); + + final var predicateMapping = predicateMappingMaybe.get(); + final var maxMatchMap = + MaxMatchMap.compute(translatedResultValue, candidateExpression.getResultValue(), + Quantifiers.aliases(candidateExpression.getQuantifiers()), ValueEquivalence.fromAliasMap(bindingAliasMap)); + final var constraints = maxMatchMap.getQueryPlanConstraint(); + return MatchInfo.RegularMatchInfo.tryMerge(bindingAliasMap, partialMatchMap, ImmutableMap.of(placeholderAlias, comparisonRange), predicateMapping, maxMatchMap, GroupByMappings.empty(), ImmutableList.of(), constraints) + .map(ImmutableList::of) + .orElse(ImmutableList.of()); + } } @Nonnull @@ -237,4 +329,34 @@ public PlannerGraph rewritePlannerGraph(@Nonnull List ch ImmutableMap.of("types", Attribute.gml(getRecordTypes().stream().map(Attribute::gml).collect(ImmutableList.toImmutableList())))), childGraphs); } + + @Nonnull + public List getPredicates() { + return predicates; + } + + @Nonnull + public static LogicalTypeFilterExpression newInstance(@Nonnull final Set recordTypes, @Nonnull final Quantifier innerQuantifier, + @Nonnull final Type resultType) { + return new LogicalTypeFilterExpression(recordTypes, innerQuantifier, resultType, ImmutableList.of()); + } + + @Nonnull + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") + public static LogicalTypeFilterExpression newInstanceForMatchCandidate(@Nonnull final Set recordTypes, + @Nonnull final Set availableRecordTypes, + @Nonnull final Quantifier innerQuantifier, + @Nonnull final Optional recordTypeKeyAliasMaybe, + @Nonnull final Type resultType) { + Verify.verify(availableRecordTypes.containsAll(recordTypes)); + + final var predicateListBuilder = ImmutableList.builder(); + if (recordTypes.size() < availableRecordTypes.size() && recordTypeKeyAliasMaybe.isPresent()) { + final var recordTypeValue = new RecordTypeValue(QuantifiedRecordValue.of(innerQuantifier)); + final var placeholder = recordTypeValue.asPlaceholder(recordTypeKeyAliasMaybe.get()); + predicateListBuilder.add(placeholder); + } + + return new LogicalTypeFilterExpression(recordTypes, innerQuantifier, resultType, predicateListBuilder.build()); + } } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/RelationalExpression.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/RelationalExpression.java index b4b6044ef4..927260fb64 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/RelationalExpression.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/RelationalExpression.java @@ -139,7 +139,7 @@ static RelationalExpression fromRecordQuery(@Nonnull RecordMetaData recordMetaDa new Type.AnyRecord(false), new AccessHints())); baseRef = Reference.initialOf( - new LogicalTypeFilterExpression( + LogicalTypeFilterExpression.newInstance( new HashSet<>(queriedRecordTypes), Quantifier.forEach(fuseRef), recordMetaData.getPlannerType(queriedRecordTypes))); diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/Placeholder.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/Placeholder.java index d612bbc0f5..d23b961e02 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/Placeholder.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/Placeholder.java @@ -62,6 +62,14 @@ public PredicateWithValueAndRanges withValue(@Nonnull final Value value) { return new Placeholder(value, getRanges(), parameterAlias); } + @Nonnull + public Placeholder withAlias(@Nonnull final CorrelationIdentifier newParameterAlias) { + if (newParameterAlias.equals(parameterAlias)) { + return this; + } + return new Placeholder(getValue(), getRanges(), newParameterAlias); + } + @Nonnull @Override public PredicateWithValueAndRanges withRanges(@Nonnull final Set ranges) { diff --git a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/query/FDBModificationQueryTest.java b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/query/FDBModificationQueryTest.java index e5af3d3049..a13724386b 100644 --- a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/query/FDBModificationQueryTest.java +++ b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/query/FDBModificationQueryTest.java @@ -124,7 +124,7 @@ public void testPlanDeleteExpression() throws Exception { new AccessHints()))); qun = Quantifier.forEach(Reference.initialOf( - new LogicalTypeFilterExpression(ImmutableSet.of("RestaurantRecord"), + LogicalTypeFilterExpression.newInstance(ImmutableSet.of("RestaurantRecord"), qun, restaurantType))); @@ -434,7 +434,7 @@ private static Reference selectRecordsGraph(Function attributesBuilder = ImmutableList.builder(); int colCount = 0; diff --git a/yaml-tests/src/test/java/YamlIntegrationTests.java b/yaml-tests/src/test/java/YamlIntegrationTests.java index aff182c251..12128c1ada 100644 --- a/yaml-tests/src/test/java/YamlIntegrationTests.java +++ b/yaml-tests/src/test/java/YamlIntegrationTests.java @@ -22,6 +22,7 @@ import com.apple.foundationdb.relational.yamltests.YamlTest; import com.apple.foundationdb.relational.yamltests.YamlTestConfigFilters; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestTemplate; /** @@ -277,6 +278,12 @@ public void pseudoFieldClash(YamlTest.Runner runner) throws Exception { runner.runYamsql("pseudo-field-clash.yamsql"); } + @TestTemplate + @MaintainYamlTestConfig(YamlTestConfigFilters.CORRECT_EXPLAIN_AND_METRICS) + public void recordTypeKeyTest(YamlTest.Runner runner) throws Exception { + runner.runYamsql("record-type-key-tests.yamsql"); + } + @TestTemplate public void recursiveCte(YamlTest.Runner runner) throws Exception { runner.runYamsql("recursive-cte.yamsql"); diff --git a/yaml-tests/src/test/resources/record-type-key-tests.metrics.binpb b/yaml-tests/src/test/resources/record-type-key-tests.metrics.binpb new file mode 100644 index 0000000000..0f0ebd1dd4 --- /dev/null +++ b/yaml-tests/src/test/resources/record-type-key-tests.metrics.binpb @@ -0,0 +1,15 @@ +› +2 + unnamed-2%EXPLAIN SELECT * from t1 where a = 10ä + +»û†ÜH. ‰‘—:(0±³‡8@7SCAN([IS T1, EQUALS promote(@c8 AS LONG)]) | TFILTER T1Œ +digraph G { + fontname=courier; + rankdir=BT; + splines=line; + 1 [ label=<
Type Filter
WHERE record IS [T1]
> color="black" shape="plain" style="solid" fillcolor="black" fontname="courier" fontsize="8" tooltip="RELATION(LONG AS A, LONG AS B)" ]; + 2 [ label=<
Scan
comparisons: [IS T1, EQUALS promote(@c8 AS LONG)]
> color="black" shape="plain" style="solid" fillcolor="black" fontname="courier" fontsize="8" tooltip="RELATION(RECORD)" ]; + 3 [ label=<
Primary Storage
record types: [T1, T2]
> color="black" shape="plain" style="filled" fillcolor="lightblue" fontname="courier" fontsize="8" tooltip="RELATION(RECORD)" ]; + 3 -> 2 [ color="gray20" style="solid" fontname="courier" fontsize="8" arrowhead="normal" arrowtail="none" dir="both" ]; + 2 -> 1 [ label=< q28> label="q28" color="gray20" style="bold" fontname="courier" fontsize="8" arrowhead="normal" arrowtail="none" dir="both" ]; +} \ No newline at end of file diff --git a/yaml-tests/src/test/resources/record-type-key-tests.metrics.yaml b/yaml-tests/src/test/resources/record-type-key-tests.metrics.yaml new file mode 100644 index 0000000000..b95f93781a --- /dev/null +++ b/yaml-tests/src/test/resources/record-type-key-tests.metrics.yaml @@ -0,0 +1,12 @@ +unnamed-2: +- query: EXPLAIN SELECT * from t1 where a = 10 + ref: record-type-key-tests.yamsql:34 + explain: SCAN([IS T1, EQUALS promote(@c8 AS LONG)]) | TFILTER T1 + task_count: 187 + task_total_time_ms: 152 + transform_count: 46 + transform_time_ms: 122 + transform_yield_count: 15 + insert_time_ms: 2 + insert_new_count: 16 + insert_reused_count: 1 diff --git a/yaml-tests/src/test/resources/record-type-key-tests.yamsql b/yaml-tests/src/test/resources/record-type-key-tests.yamsql new file mode 100644 index 0000000000..4374628afd --- /dev/null +++ b/yaml-tests/src/test/resources/record-type-key-tests.yamsql @@ -0,0 +1,36 @@ +# +# record-type-key-tests.yamsql +# +# This source file is part of the FoundationDB open source project +# +# Copyright 2021-2024 Apple Inc. and the FoundationDB project authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +--- +schema_template: + CREATE TABLE t1 (a bigint, b bigint, PRIMARY KEY(a)) + CREATE TABLE t2 (c bigint, d bigint, primary key(c)) + WITH OPTIONS(INTERMINGLE_TABLES=false) +--- +setup: + steps: + - query: insert into t1 values (10, 20) + - query: insert into t2 values (100, 200) +--- +test_block: + tests: + - + - query: SELECT * from t1 where a = 10 + - explain: "SCAN([IS T1, EQUALS promote(@c8 AS LONG)]) | TFILTER T1" + - result: [{10, 20}] +...