From f63d3f4e7c76fae34f5ceaab4fa6daf346cb1ac5 Mon Sep 17 00:00:00 2001 From: Youssef Hatem Date: Mon, 17 Oct 2022 16:08:53 +0100 Subject: [PATCH] * fix failing tests. --- .../record/query/plan/RecordQueryPlanner.java | 4 +- .../matching/structure/ValueMatchers.java | 3 +- .../rules/ImplementNestedLoopJoinRule.java | 17 ++-- .../plan/cascades/values/FieldValue.java | 10 ++- .../ComposeFieldValueOverFieldValueRule.java | 7 +- ...seFieldValueOverRecordConstructorRule.java | 10 +-- .../MatchOrCompensateFieldValueRule.java | 3 +- .../query/plan/cascades/OrderingTest.java | 78 +++++++++---------- 8 files changed, 67 insertions(+), 65 deletions(-) diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/RecordQueryPlanner.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/RecordQueryPlanner.java index a69fc2721c..6ae5948e5e 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/RecordQueryPlanner.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/RecordQueryPlanner.java @@ -1736,7 +1736,7 @@ public RecordQueryCoveringIndexPlan planCoveringAggregateIndex(@Nonnull RecordQu return null; } - final IndexKeyValueToPartialRecord.Builder builder = IndexKeyValueToPartialRecord.newBuilder(recordType); + final IndexKeyValueToPartialRecord.Builder builder = IndexKeyValueToPartialRecord.newBuilder(recordType); final List keyFields = index.getRootExpression().normalizeKeyForPositions(); final List valueFields = Collections.emptyList(); for (KeyExpression resultField : query.getRequiredResults()) { @@ -1756,7 +1756,7 @@ public RecordQueryCoveringIndexPlan planCoveringAggregateIndex(@Nonnull RecordQu } private static boolean addCoveringField(@Nonnull KeyExpression requiredExpr, - @Nonnull IndexKeyValueToPartialRecord.Builder builder, + @Nonnull IndexKeyValueToPartialRecord.Builder builder, @Nonnull List keyFields, @Nonnull List valueFields) { final IndexKeyValueToPartialRecord.TupleSource source; diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/matching/structure/ValueMatchers.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/matching/structure/ValueMatchers.java index 602f1513e5..e16bd06a8e 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/matching/structure/ValueMatchers.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/matching/structure/ValueMatchers.java @@ -22,7 +22,6 @@ import com.apple.foundationdb.annotation.API; import com.apple.foundationdb.record.query.plan.cascades.Quantifier; -import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.apple.foundationdb.record.query.plan.cascades.values.ArithmeticValue; import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue; import com.apple.foundationdb.record.query.plan.cascades.values.NumericAggregationValue; @@ -90,7 +89,7 @@ public static BindingMatcher fieldValueWithFieldNa @Nonnull public static BindingMatcher fieldValueWithFieldPath(@Nonnull final BindingMatcher downstreamValue, - @Nonnull final CollectionMatcher downstreamFieldPath) { + @Nonnull final CollectionMatcher downstreamFieldPath) { final TypedMatcherWithExtractAndDownstream downstreamValueMatcher = typedWithDownstream(FieldValue.class, Extractor.of(FieldValue::getChild, name -> "child(" + name + ")"), diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/rules/ImplementNestedLoopJoinRule.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/rules/ImplementNestedLoopJoinRule.java index 64f689d2e5..67d7a2c4a2 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/rules/ImplementNestedLoopJoinRule.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/rules/ImplementNestedLoopJoinRule.java @@ -99,9 +99,14 @@ public class ImplementNestedLoopJoinRule extends CascadesRule @Nonnull private static final String OUTER_FIELD_NAME = "outer"; + + private static final int OUTER_FIELD_INDEX = 0; + @Nonnull private static final String INNER_FIELD_NAME = "inner"; + private static final int INNER_FIELD_INDEX = 1; + public ImplementNestedLoopJoinRule() { // TODO figure out which constraints this rule should be sensitive to super(root, ImmutableSet.of(RequestedOrderingConstraint.REQUESTED_ORDERING)); @@ -301,8 +306,8 @@ public void onMatch(@Nonnull final CascadesRuleCall call) { final var joinedResultValue = RecordConstructorValue.ofColumns( ImmutableList.of( - Column.of(Type.Record.Field.of(outerValue.getResultType(), Optional.of(OUTER_FIELD_NAME)), outerValue), - Column.of(Type.Record.Field.of(innerValue.getResultType(), Optional.of(INNER_FIELD_NAME)), innerValue))); + Column.of(Type.Record.Field.of(outerValue.getResultType(), Optional.of(OUTER_FIELD_NAME), Optional.of(OUTER_FIELD_INDEX)), outerValue), + Column.of(Type.Record.Field.of(innerValue.getResultType(), Optional.of(INNER_FIELD_NAME), Optional.of(INNER_FIELD_INDEX)), innerValue))); final var joinedAlias = Quantifier.uniqueID(); final var joinedQuantifier = @@ -317,9 +322,9 @@ public void onMatch(@Nonnull final CascadesRuleCall call) { final var translationMap = TranslationMap.builder() .when(outerAlias).then(joinedAlias, (sourceAlias, targetAlias, leafValue) -> - replaceJoinedReference(targetAlias, joinedResultValue.getResultType(), OUTER_FIELD_NAME, leafValue)) + replaceJoinedReference(targetAlias, joinedResultValue.getResultType(), OUTER_FIELD_INDEX, leafValue)) .when(innerAlias).then(joinedAlias, (sourceAlias, targetAlias, leafValue) -> - replaceJoinedReference(targetAlias, joinedResultValue.getResultType(), INNER_FIELD_NAME, leafValue)) + replaceJoinedReference(targetAlias, joinedResultValue.getResultType(), INNER_FIELD_INDEX, leafValue)) .build(); @@ -345,9 +350,9 @@ public void onMatch(@Nonnull final CascadesRuleCall call) { @Nonnull private static Value replaceJoinedReference(@Nonnull final CorrelationIdentifier resultAlias, @Nonnull final Type.Record joinedRecordType, - @Nonnull final String fieldName, + final int fieldIndex, @Nonnull final LeafValue leafValue) { - final var fieldValue = FieldValue.ofFieldName(QuantifiedObjectValue.of(resultAlias, joinedRecordType), fieldName); + final var fieldValue = FieldValue.ofFieldIndex(QuantifiedObjectValue.of(resultAlias, joinedRecordType), fieldIndex); return leafValue.replaceReferenceWithField(fieldValue); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/FieldValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/FieldValue.java index 2ac9291060..315e529313 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/FieldValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/FieldValue.java @@ -200,11 +200,17 @@ private static FieldPath resolveFieldPath(@Nonnull final Type inputType, @Nonnul return new FieldPath(accessorPathBuilder.build().stream().map(FieldDelegate::of).collect(Collectors.toList())); } + @VisibleForTesting @Nonnull public static FieldValue ofFieldName(@Nonnull Value childValue, @Nonnull final String fieldName) { return new FieldValue(childValue, resolveFieldPath(childValue.getResultType(), ImmutableList.of(new Accessor(fieldName, -1)))); } + @Nonnull + public static FieldValue ofFieldIndex(@Nonnull Value childValue, @Nonnull final int fieldIndex) { + return new FieldValue(childValue, resolveFieldPath(childValue.getResultType(), ImmutableList.of(new Accessor(null, fieldIndex)))); + } + public static FieldValue ofFieldNames(@Nonnull Value childValue, @Nonnull final List fieldNames) { return new FieldValue(childValue, resolveFieldPath(childValue.getResultType(), fieldNames.stream().map(fieldName -> new Accessor(fieldName, -1)).collect(ImmutableList.toImmutableList()))); } @@ -213,10 +219,6 @@ public static FieldValue ofAccessors(@Nonnull Value childValue, @Nonnull final L return new FieldValue(childValue, resolveFieldPath(childValue.getResultType(), accessors)); } - public static FieldValue ofFields(@Nonnull Value childValue, @Nonnull final List fields) { - return new FieldValue(childValue, new FieldPath(fields.stream().map(FieldDelegate::of).collect(Collectors.toList()))); - } - public static FieldValue ofFieldDelegates(@Nonnull Value childValue, @Nonnull final List fieldDelegates) { return new FieldValue(childValue, new FieldPath(fieldDelegates)); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/ComposeFieldValueOverFieldValueRule.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/ComposeFieldValueOverFieldValueRule.java index effc2d8eaa..b18c718937 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/ComposeFieldValueOverFieldValueRule.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/ComposeFieldValueOverFieldValueRule.java @@ -24,7 +24,6 @@ import com.apple.foundationdb.record.query.plan.cascades.matching.structure.BindingMatcher; import com.apple.foundationdb.record.query.plan.cascades.matching.structure.CollectionMatcher; import com.apple.foundationdb.record.query.plan.cascades.matching.structure.ValueMatchers; -import com.apple.foundationdb.record.query.plan.cascades.typing.Type.Record.Field; import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue; import com.apple.foundationdb.record.query.plan.cascades.values.Value; import com.google.common.base.Verify; @@ -48,14 +47,14 @@ public class ComposeFieldValueOverFieldValueRule extends ValueSimplificationRule @Nonnull private static final BindingMatcher innerChildMatcher = anyValue(); @Nonnull - private static final CollectionMatcher innerFieldPathMatcher = all(anyObject()); + private static final CollectionMatcher innerFieldPathMatcher = all(anyObject()); @Nonnull private static final BindingMatcher innerFieldValueMatcher = ValueMatchers.fieldValueWithFieldPath(innerChildMatcher, innerFieldPathMatcher); @Nonnull - private static final CollectionMatcher outerFieldPathMatcher = all(anyObject()); + private static final CollectionMatcher outerFieldPathMatcher = all(anyObject()); @Nonnull private static final BindingMatcher rootMatcher = @@ -75,6 +74,6 @@ public void onMatch(@Nonnull final ValueSimplificationRuleCall call) { final var outerFieldPath = bindings.get(outerFieldPathMatcher); Verify.verify(!outerFieldPath.isEmpty()); - call.yield(FieldValue.ofFields(innerChild, ImmutableList.builder().addAll(innerFieldPath).addAll(outerFieldPath).build())); + call.yield(FieldValue.ofFieldDelegates(innerChild, ImmutableList.builder().addAll(innerFieldPath).addAll(outerFieldPath).build())); } } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/ComposeFieldValueOverRecordConstructorRule.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/ComposeFieldValueOverRecordConstructorRule.java index fe7ba53566..c719d71097 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/ComposeFieldValueOverRecordConstructorRule.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/ComposeFieldValueOverRecordConstructorRule.java @@ -27,7 +27,6 @@ import com.apple.foundationdb.record.query.plan.cascades.matching.structure.BindingMatcher; import com.apple.foundationdb.record.query.plan.cascades.matching.structure.CollectionMatcher; import com.apple.foundationdb.record.query.plan.cascades.matching.structure.ValueMatchers; -import com.apple.foundationdb.record.query.plan.cascades.typing.Type.Record.Field; import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue; import com.apple.foundationdb.record.query.plan.cascades.values.RecordConstructorValue; import com.apple.foundationdb.record.query.plan.cascades.values.Value; @@ -54,7 +53,7 @@ public class ComposeFieldValueOverRecordConstructorRule extends ValueSimplificat recordConstructorValue(all(anyValue())); @Nonnull - private static final CollectionMatcher fieldPathMatcher = all(anyObject()); + private static final CollectionMatcher fieldPathMatcher = all(anyObject()); @Nonnull private static final BindingMatcher rootMatcher = @@ -79,7 +78,7 @@ public void onMatch(@Nonnull final ValueSimplificationRuleCall call) { // just return the child call.yield(column.getValue()); } else { - call.yield(FieldValue.ofFields(column.getValue(), + call.yield(FieldValue.ofFieldDelegates(column.getValue(), fieldPath.stream() .skip(1L) .collect(ImmutableList.toImmutableList()))); @@ -87,13 +86,12 @@ public void onMatch(@Nonnull final ValueSimplificationRuleCall call) { } @Nonnull - private static Column findColumn(@Nonnull final RecordConstructorValue recordConstructorValue, @Nonnull final Field field) { + private static Column findColumn(@Nonnull final RecordConstructorValue recordConstructorValue, @Nonnull final FieldValue.FieldDelegate field) { for (final var column : recordConstructorValue.getColumns()) { if (field.getFieldIndex() == column.getField().getFieldIndex()) { - Verify.verify(field.getFieldNameOptional().equals(column.getField().getFieldNameOptional())); return column; } } - throw new RecordCoreException("should have found field by field name"); + throw new RecordCoreException(String.format("could not find field with index #%d", field.getFieldIndex())); } } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/MatchOrCompensateFieldValueRule.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/MatchOrCompensateFieldValueRule.java index 24ae139903..2dd5c59a81 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/MatchOrCompensateFieldValueRule.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/simplification/MatchOrCompensateFieldValueRule.java @@ -25,7 +25,6 @@ import com.apple.foundationdb.record.query.plan.cascades.matching.structure.BindingMatcher; import com.apple.foundationdb.record.query.plan.cascades.matching.structure.CollectionMatcher; import com.apple.foundationdb.record.query.plan.cascades.matching.structure.ValueMatchers; -import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue; import com.apple.foundationdb.record.query.plan.cascades.values.Value; import com.google.common.collect.ImmutableList; @@ -47,7 +46,7 @@ @SuppressWarnings("PMD.TooManyStaticImports") public class MatchOrCompensateFieldValueRule extends ValueComputationRule, Map>, FieldValue> { @Nonnull - private static final CollectionMatcher fieldPathMatcher = all(anyObject()); + private static final CollectionMatcher fieldPathMatcher = all(anyObject()); @Nonnull private static final BindingMatcher rootMatcher = diff --git a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/OrderingTest.java b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/OrderingTest.java index 1e507c5a61..7ede332ad2 100644 --- a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/OrderingTest.java +++ b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/OrderingTest.java @@ -43,9 +43,9 @@ class OrderingTest { @Test void testOrdering() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); final var requestedOrdering = new RequestedOrdering(ImmutableList.of(a, b, c), RequestedOrdering.Distinctness.NOT_DISTINCT); @@ -64,9 +64,9 @@ void testOrdering() { @Test void testOrdering2() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); final var requestedOrdering = new RequestedOrdering(ImmutableList.of(a, b, c), RequestedOrdering.Distinctness.NOT_DISTINCT); @@ -86,9 +86,9 @@ void testOrdering2() { @Test void testMergeKeys() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); final var leftPartialOrder = PartiallyOrderedSet.of(ImmutableSet.of(a, b, c), @@ -108,9 +108,9 @@ void testMergeKeys() { @Test void testMergeKeys2() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); final var leftPartialOrder = PartiallyOrderedSet.of(ImmutableSet.of(a, b, c), @@ -129,9 +129,9 @@ void testMergeKeys2() { @Test void testMergeKeys3() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); final var leftPartialOrder = PartiallyOrderedSet.of(ImmutableSet.of(a, b, c), @@ -148,11 +148,11 @@ void testMergeKeys3() { @Test void testMergePartialOrdersNAry() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); - final var d = of(field("d")); - final var e = of(field("e")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); + final var d = of(field("d", 4)); + final var e = of(field("e", 5)); final var one = PartiallyOrderedSet.of(ImmutableSet.of(a, b, c, d), @@ -180,11 +180,11 @@ void testMergePartialOrdersNAry() { @Test void testCommonOrdering() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); - final var d = of(field("d")); - final var e = of(field("e")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); + final var d = of(field("d", 4)); + final var e = of(field("e", 5)); final var one = new Ordering( ImmutableSetMultimap.of(d.getValue(), new Comparisons.NullComparison(Comparisons.Type.IS_NULL)), @@ -223,10 +223,10 @@ void testCommonOrdering() { @Test void testCommonOrdering2() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); - final var x = of(field("x")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); + final var x = of(field("x", 4)); final var one = new Ordering( ImmutableSetMultimap.of(c.getValue(), new Comparisons.NullComparison(Comparisons.Type.IS_NULL)), @@ -254,10 +254,10 @@ void testCommonOrdering2() { @Test void testCommonOrdering3() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); - final var x = of(field("x")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); + final var x = of(field("x", 4)); final var one = new Ordering( ImmutableSetMultimap.of(c.getValue(), new Comparisons.NullComparison(Comparisons.Type.IS_NULL)), @@ -285,10 +285,10 @@ void testCommonOrdering3() { @Test void testCommonOrdering4() { - final var a = of(field("a")); - final var b = of(field("b")); - final var c = of(field("c")); - final var x = of(field("x")); + final var a = of(field("a", 1)); + final var b = of(field("b", 2)); + final var c = of(field("c", 3)); + final var x = of(field("x", 4)); final var one = new Ordering( ImmutableSetMultimap.of(c.getValue(), new Comparisons.NullComparison(Comparisons.Type.IS_NULL)), @@ -311,10 +311,10 @@ void testCommonOrdering4() { } @Nonnull - private static Value field(@Nonnull final String fieldName) { + private static Value field(@Nonnull final String fieldName, int fieldIndex) { final ImmutableList> columns = ImmutableList.of( - Column.of(Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of(fieldName)), + Column.of(Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of(fieldName), Optional.of(fieldIndex)), LiteralValue.ofScalar("fieldValue"))); return FieldValue.ofFieldName(RecordConstructorValue.ofColumns(columns), fieldName); }