diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 2b77c8f57e..b3fac7b717 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -20,7 +20,7 @@ Support for the Protobuf 2 runtime has been removed as of this version. All arti * **Bug fix** Fix 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Bug fix** Fix 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Bug fix** Fix topological sort instability [(Issue #2476)](https://github.com/FoundationDB/fdb-record-layer/pull/2476) -* **Performance** Improvement 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) +* **Performance** Cascades Pre-order Iterable is inefficient [(Issue #2481)](https://github.com/FoundationDB/fdb-record-layer/issues/2481) * **Performance** Improvement 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Performance** Improvement 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Performance** Improvement 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/metadata/IndexComparison.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/metadata/IndexComparison.java index 03e50a9f70..9112b67e45 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/metadata/IndexComparison.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/metadata/IndexComparison.java @@ -30,7 +30,6 @@ import javax.annotation.Nonnull; import java.util.Objects; -import java.util.stream.StreamSupport; /** * This is a simple PoJo hierarchy representing SerDe operations on a predicate comparison of a sparse {@link Index}. @@ -85,14 +84,12 @@ public static IndexComparison fromComparison(@Nonnull final Comparisons.Comparis */ public static boolean isSupported(@Nonnull final Comparisons.Comparison comparison) { return comparison instanceof Comparisons.SimpleComparison || - comparison instanceof Comparisons.NullComparison || - (comparison instanceof Comparisons.ValueComparison && - StreamSupport.stream(((Comparisons.ValueComparison)comparison) - .getComparandValue() - .filter(value -> !(value instanceof Value.RangeMatchableValue)) - .spliterator(), false) - .findAny() - .isEmpty()); + comparison instanceof Comparisons.NullComparison || + (comparison instanceof Comparisons.ValueComparison && + ((Comparisons.ValueComparison)comparison).getComparandValue().preOrderStream() + .filter(value -> !(value instanceof Value.RangeMatchableValue)) + .findAny() + .isEmpty()); } /** diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/NotValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/NotValue.java index 9fe5240fa1..b2d39cdd11 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/NotValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/NotValue.java @@ -39,10 +39,10 @@ import com.apple.foundationdb.record.query.plan.cascades.values.AbstractValue; import com.apple.foundationdb.record.query.plan.cascades.values.BooleanValue; import com.apple.foundationdb.record.query.plan.cascades.values.Value; +import com.apple.foundationdb.record.query.plan.cascades.values.ValueWithChild; import com.google.auto.service.AutoService; import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; import com.google.protobuf.Message; import javax.annotation.Nonnull; @@ -55,7 +55,7 @@ * A value that flips the output of its boolean child. */ @API(API.Status.EXPERIMENTAL) -public class NotValue extends AbstractValue implements BooleanValue { +public class NotValue extends AbstractValue implements BooleanValue, ValueWithChild { /** * The hash value of this expression. */ @@ -98,15 +98,20 @@ public Optional toQueryPredicate(@Nullable final TypeRepository @Nonnull @Override - public Iterable getChildren() { - return ImmutableList.of(child); + protected Iterable computeChildren() { + return ImmutableList.of(getChild()); } @Nonnull @Override - public NotValue withChildren(final Iterable newChildren) { - Verify.verify(Iterables.size(newChildren) == 1); - return new NotValue(Iterables.get(newChildren, 0)); + public Value getChild() { + return child; + } + + @Nonnull + @Override + public ValueWithChild withNewChild(@Nonnull final Value rebasedChild) { + return new NotValue(rebasedChild); } @Nullable @@ -124,7 +129,7 @@ public Object eval(@Nonnull final FDBRecordStoreBase stor public int hashCodeWithoutChildren() { return PlanHashable.objectsPlanHash(PlanHashable.CURRENT_FOR_CONTINUATION, BASE_HASH); } - + @Override public int planHash(@Nonnull final PlanHashMode mode) { return PlanHashable.objectsPlanHash(mode, BASE_HASH, child); diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PreOrderIterator.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PreOrderIterator.java new file mode 100644 index 0000000000..4da148e297 --- /dev/null +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PreOrderIterator.java @@ -0,0 +1,113 @@ +/* + * PreOrderIterator.java + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2015-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. + */ + +package com.apple.foundationdb.record.query.plan.cascades; + +import com.google.common.base.Verify; +import com.google.common.collect.Iterables; +import org.apache.commons.lang3.tuple.MutablePair; +import org.apache.commons.lang3.tuple.Pair; + +import javax.annotation.Nonnull; +import javax.annotation.concurrent.NotThreadSafe; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; + +/** + * An iterator that accesses all elements of a {@link TreeLike} object in pre-order fashion. + * It attempts to reduce the number of memory allocations while still being performant. It does so + * by implementing a mix of level-based traversal and pre-order traversal, where a {@link Deque} is used + * (as a stack) to enable depth first search, however instead of maintaining individual {@link TreeLike} node in + * each stack frame the entire list of node's children is stored instead in addition to an index pointing out + * to the current element that is returned by the stream. + * + * @param The type of the iterator element. + */ +@NotThreadSafe +public final class PreOrderIterator> implements Iterator { + + @Nonnull + private final Deque, Integer>> stack; + + private static final int INITIAL_POSITION = -1; + + private PreOrderIterator(@Nonnull final T traversable) { + // initialize the stack with the {@link TreeLike}'s depth as capacity to avoid resizing. + stack = new ArrayDeque<>(traversable.height()); + // this is the only list allocation done to put the root in the stack. + // all the remaining lists added to the stack are references to children + // lists (copy by reference). + stack.push(MutablePair.of(List.of(traversable.getThis()), INITIAL_POSITION)); + } + + @Override + public boolean hasNext() { + while (true) { + if (stack.isEmpty()) { + return false; + } + final var top = Verify.verifyNotNull(stack.peekLast()); + final var currentLevelIndex = top.getRight(); + final var currentLevelItemsCount = Iterables.size(top.getLeft()); + if (currentLevelIndex + 1 < currentLevelItemsCount) { + return true; + } else { + // pop the stack, and continue doing so, until we either find a next element + // by looking into previous levels in reverse (stack) order, progressively + // do so until either an item is found, or the stack becomes empty, and no item + // is to be found meaning that the traversal is complete. + stack.removeLast(); + } + } + } + + @Override + public T next() { + if (!hasNext()) { + throw new NoSuchElementException("no more elements"); + } + final var top = Verify.verifyNotNull(stack.peekLast()); + final var currentIndexPosition = top.getRight(); + final var currentLevelItems = top.getLeft(); + final var nextItemIndex = currentIndexPosition + 1; + + // mark the next item as the current one in this stack frame, note that + // the position must be valid because hasNext() is called first, and hasNext() + // makes sure that, if there is a next element, it modifies the stack index such + // that incrementing it would lead to finding that next element correctly. + final var result = Iterables.get(currentLevelItems, nextItemIndex); + top.setValue(nextItemIndex); + final var resultChildren = result.getChildren(); + + // descend immediately to the children (if any) so to conform to pre-order DFS semantics. + if (!Iterables.isEmpty(resultChildren)) { + stack.add(MutablePair.of(resultChildren, INITIAL_POSITION)); + } + return result; + } + + @Nonnull + public static > PreOrderIterator over(@Nonnull final T traversable) { + return new PreOrderIterator<>(traversable); + } +} diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/TreeLike.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/TreeLike.java index 5b252c975b..be802d5f09 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/TreeLike.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/TreeLike.java @@ -35,6 +35,7 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.function.UnaryOperator; +import java.util.stream.Stream; /** * Interface for tree-like structures and helpers for folds and traversals on these tree-like structures of node type @@ -69,13 +70,31 @@ public interface TreeLike> { @Nonnull T withChildren(Iterable newChildren); + /** + * Returns an iterator that traverse the nodes in pre-order. + * @return an iterator that traverse the nodes in pre-order. + */ + @Nonnull + default Iterator preOrderIterator() { + return PreOrderIterator.over(getThis()); + } + + /** + * Returns a {@link Stream} that traverses the nodes in pre-order. + * @return a {@link Stream} that traverses the nodes in pre-order. + */ + @Nonnull + default Stream preOrderStream() { + return Streams.stream(preOrderIterator()); + } + /** * Method that returns an {@link Iterable} of nodes as encountered in pre-order traversal of this tree-like. * @return an {@link Iterable} of nodes */ @Nonnull - default Iterable inPreOrder() { - return inPreOrder(self -> true); + default Iterable preOrderIterable() { + return preOrderIterable(self -> true); } /** @@ -85,13 +104,13 @@ default Iterable inPreOrder() { * @return an {@link Iterable} of nodes */ @Nonnull - default Iterable inPreOrder(@Nonnull final Predicate descentIntoPredicate) { + default Iterable preOrderIterable(@Nonnull final Predicate descentIntoPredicate) { final ImmutableList.Builder> iterablesBuilder = ImmutableList.builder(); final var self = getThis(); iterablesBuilder.add(ImmutableList.of(self)); if (descentIntoPredicate.test(self)) { for (final T child : getChildren()) { - iterablesBuilder.add(child.inPreOrder()); + iterablesBuilder.add(child.preOrderIterable()); } } return Iterables.concat(iterablesBuilder.build()); @@ -102,40 +121,15 @@ default Iterable inPreOrder(@Nonnull final Predicate descentInto * @return an {@link Iterable} of nodes */ @Nonnull - default Iterable inPostOrder() { + default Iterable postOrderIterable() { final ImmutableList.Builder> iterablesBuilder = ImmutableList.builder(); for (final T child : getChildren()) { - iterablesBuilder.add(child.inPostOrder()); + iterablesBuilder.add(child.postOrderIterable()); } iterablesBuilder.add(ImmutableList.of(getThis())); return Iterables.concat(iterablesBuilder.build()); } - /** - * Method that returns an {@link Iterable} of nodes as encountered in pre-order traversal of this tree-like that - * are filtered by a {@link Predicate} that filters out every node for which {@code predicate} returns {@code false}. - * @param predicate a {@link Predicate} that is evaluated per encountered node during pre-order traversal of - * the tree-like rooted at {@code this} - * @return an {@link Iterable} of nodes satisfying the predicate passed in - */ - @Nonnull - default Iterable filter(@Nonnull final Predicate predicate) { - return Iterables.filter(inPreOrder(), predicate::test); - } - - /** - * Method that returns an {@link Iterable} of nodes as encountered in pre-order traversal of this tree-like that - * are filtered by a {@link Predicate} that filters out every node for which {@code predicate} returns {@code false}. - * @param a fixed sub class of {@code T} - * @param clazz a class object that is used to filter the nodes in this tree-like by their associated class - * @return an {@link Iterable} of nodes that are at least of type {@code T1} - */ - @Nonnull - @SuppressWarnings("unchecked") - default Iterable filter(@Nonnull final Class clazz) { - return () -> Streams.stream(inPreOrder()).filter(clazz::isInstance).map(t -> (T1)t).iterator(); - } - /** * Method that performs a tree fold over the tree-like rooted at {@code this} using the given map and fold functions. * Note that this variant of fold does not permit {@code null} values to be produced and consumed by the map and @@ -252,6 +246,14 @@ default Optional replaceLeavesMaybe(@Nonnull final UnaryOperator replaceOp }); } + /** + * returns the height of the tree. + * @return the height of the tree. + */ + default int height() { + return Streams.stream(getChildren()).mapToInt(TreeLike::height).max().orElse(0) + 1; + } + /** * Functional interface for a function whose result is not nullable. * @param argument type diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/RelationalExpressionWithPredicates.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/RelationalExpressionWithPredicates.java index fb83fa260b..09c876b801 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/RelationalExpressionWithPredicates.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/RelationalExpressionWithPredicates.java @@ -35,7 +35,6 @@ import java.util.List; import java.util.Set; import java.util.function.Predicate; -import java.util.stream.StreamSupport; /** * A (relational) expression that has a predicate on it. @@ -99,16 +98,11 @@ static ImmutableSet fieldValuesFromPredicates(@Nonnull final Collect @Nonnull final Predicate filteringPredicate) { return predicates .stream() - .flatMap(predicate -> { - final Iterable filters = - predicate.filter(p -> p instanceof PredicateWithValue && filteringPredicate.test((PredicateWithValue)p)); - return StreamSupport.stream(filters.spliterator(), false) - .map(p -> (PredicateWithValue)p) - .flatMap(predicateWithValue -> - StreamSupport.stream(predicateWithValue.getValue() - .filter(v -> v instanceof FieldValue).spliterator(), false)) - .map(value -> (FieldValue)value); - }) + .flatMap(predicate -> predicate.preOrderStream() + .filter(p -> p instanceof PredicateWithValue && filteringPredicate.test((PredicateWithValue)p)) + .map(p -> (PredicateWithValue)p) + .flatMap(predicateWithValue -> predicateWithValue.getValue().preOrderStream().filter(FieldValue.class::isInstance)) + .map(value -> (FieldValue)value)) .map(fieldValue -> { final Set fieldCorrelatedTo = fieldValue.getChild().getCorrelatedTo(); // TODO make better as the field can currently only handle exactly one correlated alias diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/AbstractQueryPredicate.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/AbstractQueryPredicate.java index f148b07874..2e79e9cb65 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/AbstractQueryPredicate.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/AbstractQueryPredicate.java @@ -45,6 +45,8 @@ public abstract class AbstractQueryPredicate implements QueryPredicate { private final Supplier semanticHashCodeSupplier; + private final Supplier heightSupplier; + @SuppressWarnings("unused") protected AbstractQueryPredicate(@Nonnull final PlanSerializationContext serializationContext, @Nonnull final PAbstractQueryPredicate abstractQueryPredicateProto) { @@ -56,6 +58,7 @@ protected AbstractQueryPredicate(final boolean isAtomic) { this.isAtomic = isAtomic; this.correlatedToSupplier = Suppliers.memoize(this::computeCorrelatedTo); this.semanticHashCodeSupplier = Suppliers.memoize(this::computeSemanticHashCode); + this.heightSupplier = Suppliers.memoize(QueryPredicate.super::height); } @Nonnull @@ -88,6 +91,11 @@ public int semanticHashCode() { protected abstract int computeSemanticHashCode(); + @Override + public int height() { + return heightSupplier.get(); + } + @Override public int hashCodeWithoutChildren() { return Objects.hash(isAtomic); diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/OrPredicate.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/OrPredicate.java index e710966fba..12e91374ef 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/OrPredicate.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/predicates/OrPredicate.java @@ -40,7 +40,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.common.collect.Streams; import com.google.protobuf.Message; import org.checkerframework.checker.nullness.qual.NonNull; @@ -228,16 +227,15 @@ public Optional impliesCandidatePredicate(@N if (mappingsOptional.isEmpty() && candidatePredicate instanceof Placeholder) { final var candidateValue = ((Placeholder)candidatePredicate).getValue(); - final var anyMatchingLeafPredicate = - Streams.stream(inPreOrder()) - .filter(predicate -> predicate instanceof LeafQueryPredicate) - .anyMatch(predicate -> { - if (predicate instanceof PredicateWithValue) { - final var queryValue = ((ValuePredicate)predicate).getValue(); - return queryValue.semanticEquals(candidateValue, aliasMap); - } - return false; - }); + final var anyMatchingLeafPredicate = preOrderStream() + .filter(LeafQueryPredicate.class::isInstance) + .anyMatch(predicate -> { + if (predicate instanceof PredicateWithValue) { + final var queryValue = ((ValuePredicate)predicate).getValue(); + return queryValue.semanticEquals(candidateValue, aliasMap); + } + return false; + }); if (anyMatchingLeafPredicate) { // // There is a sub-term that could be matched if the OR was broken into a UNION. Mark this as a diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PushReferencedFieldsThroughSelectRule.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PushReferencedFieldsThroughSelectRule.java index 47964d3aa5..0b50bf21c9 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PushReferencedFieldsThroughSelectRule.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/rules/PushReferencedFieldsThroughSelectRule.java @@ -41,7 +41,6 @@ import javax.annotation.Nonnull; import java.util.List; import java.util.Set; -import java.util.stream.StreamSupport; import static com.apple.foundationdb.record.query.plan.cascades.matching.structure.ListMatcher.exactly; import static com.apple.foundationdb.record.query.plan.cascades.matching.structure.MultiMatcher.all; @@ -95,10 +94,8 @@ public void onMatch(@Nonnull final CascadesRuleCall call) { * @return a set of {@link FieldValue}s */ private ImmutableSet getFieldValuesFromResultValues(@Nonnull final SelectExpression selectExpression) { - return StreamSupport.stream(selectExpression - .getResultValue() - .filter(value -> value instanceof FieldValue) - .spliterator(), false) + return selectExpression.getResultValue().preOrderStream() + .filter(FieldValue.class::isInstance) .map(value -> (FieldValue)value) .collect(ImmutableSet.toImmutableSet()); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AbstractArrayConstructorValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AbstractArrayConstructorValue.java index 417687b42f..4c2fee3843 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AbstractArrayConstructorValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AbstractArrayConstructorValue.java @@ -46,7 +46,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -93,7 +92,7 @@ public Type getResultType() { @Nonnull @Override - public Collection getChildren() { + protected Iterable computeChildren() { return children; } @@ -201,7 +200,7 @@ private LightArrayConstructorValue(@Nonnull final List children @Override @SuppressWarnings("java:S6213") public Object eval(@Nonnull final FDBRecordStoreBase store, @Nonnull final EvaluationContext context) { - return getChildren().stream() + return Streams.stream(getChildren()) .map(child -> child.eval(store, context)) .collect(ImmutableList.toImmutableList()); } @@ -218,7 +217,7 @@ public LightArrayConstructorValue withChildren(final Iterable n @Override public boolean canResultInType(@Nonnull final Type type) { - if (!getChildren().isEmpty()) { + if (!Iterables.isEmpty(getChildren())) { return false; } return type.isUnresolved(); @@ -227,7 +226,7 @@ public boolean canResultInType(@Nonnull final Type type) { @Nonnull @Override public Value with(@Nonnull final Type type) { - Verify.verify(getChildren().isEmpty()); + Verify.verify(Iterables.isEmpty(getChildren())); return emptyArray(type); // only empty arrays are currently promotable } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AbstractValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AbstractValue.java index 7996300d9f..e3bf13125d 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AbstractValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AbstractValue.java @@ -36,13 +36,23 @@ @API(API.Status.EXPERIMENTAL) public abstract class AbstractValue implements Value { + @Nonnull private final Supplier> correlatedToSupplier; + @Nonnull private final Supplier semanticHashCodeSupplier; + @Nonnull + private final Supplier heightSupplier; + + @Nonnull + private final Supplier> childrenSupplier; + protected AbstractValue() { this.correlatedToSupplier = Suppliers.memoize(this::computeCorrelatedTo); this.semanticHashCodeSupplier = Suppliers.memoize(this::computeSemanticHashCode); + this.heightSupplier = Suppliers.memoize(Value.super::height); + this.childrenSupplier = Suppliers.memoize(this::computeChildren); } @Nonnull @@ -77,4 +87,18 @@ private int computeSemanticHashCode() { return fold(Value::hashCodeWithoutChildren, (hashCodeWithoutChildren, childrenHashCodes) -> Objects.hash(childrenHashCodes, hashCodeWithoutChildren)); } + + @Override + public int height() { + return heightSupplier.get(); + } + + @Nonnull + @Override + public Iterable getChildren() { + return childrenSupplier.get(); + } + + @Nonnull + protected abstract Iterable computeChildren(); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AndOrValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AndOrValue.java index b013f0f4c7..6e38915ada 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AndOrValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/AndOrValue.java @@ -140,7 +140,7 @@ public String explain(@Nonnull final Formatter formatter) { @Nonnull @Override - public Iterable getChildren() { + protected Iterable computeChildren() { return ImmutableList.of(leftChild, rightChild); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ArithmeticValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ArithmeticValue.java index 3cb3cd9169..4cadbe1f57 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ArithmeticValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ArithmeticValue.java @@ -120,7 +120,7 @@ public Type getResultType() { @Nonnull @Override - public Iterable getChildren() { + protected Iterable computeChildren() { return ImmutableList.of(leftChild, rightChild); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConditionSelectorValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConditionSelectorValue.java index 987123592c..1372b5beaf 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConditionSelectorValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConditionSelectorValue.java @@ -55,7 +55,7 @@ public ConditionSelectorValue(@Nonnull final Iterable implicati @Nonnull @Override - public Iterable getChildren() { + protected Iterable computeChildren() { return implications; } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConstantObjectValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConstantObjectValue.java index 2bcaa6a4e7..ff4bbb3801 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConstantObjectValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConstantObjectValue.java @@ -36,6 +36,7 @@ import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.google.auto.service.AutoService; import com.google.common.base.Verify; +import com.google.common.collect.ImmutableList; import com.google.protobuf.DynamicMessage; import com.google.protobuf.Message; @@ -83,6 +84,12 @@ public Set getCorrelatedToWithoutChildren() { return Set.of(); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + @Override @SpotBugsSuppressWarnings("EQ_UNUSUAL") @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConstantValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConstantValue.java index eadbe34f85..908cc9b8af 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConstantValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ConstantValue.java @@ -35,6 +35,7 @@ import com.apple.foundationdb.record.query.plan.cascades.Formatter; import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; import com.google.protobuf.Message; import javax.annotation.Nonnull; @@ -47,6 +48,7 @@ */ @API(API.Status.EXPERIMENTAL) public class ConstantValue extends AbstractValue implements LeafValue { + private static final ObjectPlanHash BASE_HASH = new ObjectPlanHash("Constant-Value"); @Nonnull @@ -73,6 +75,12 @@ public Set getCorrelatedToWithoutChildren() { return value.getCorrelatedTo(); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + @Nullable @Override public Object eval(@Nonnull final FDBRecordStoreBase store, @Nonnull final EvaluationContext context) { diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/CountValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/CountValue.java index 8e0bd78201..bd82680151 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/CountValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/CountValue.java @@ -136,7 +136,7 @@ public Type getResultType() { @Nonnull @Override - public Iterable getChildren() { + protected Iterable computeChildren() { if (child != null) { return ImmutableList.of(child); } else { diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/DerivedValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/DerivedValue.java index 5a1f113dee..4380b80d96 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/DerivedValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/DerivedValue.java @@ -64,7 +64,7 @@ public DerivedValue(@Nonnull Iterable values, @Nonnull Type res @Nonnull @Override - public Iterable getChildren() { + protected Iterable computeChildren() { return children; } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/EmptyValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/EmptyValue.java index a054dcf2fd..d19e0641c5 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/EmptyValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/EmptyValue.java @@ -33,6 +33,7 @@ import com.apple.foundationdb.record.provider.foundationdb.FDBRecordStoreBase; import com.apple.foundationdb.record.query.plan.cascades.AliasMap; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; import com.google.protobuf.Message; import javax.annotation.Nonnull; @@ -114,6 +115,12 @@ public static EmptyValue fromProto(@Nonnull final PlanSerializationContext seria return new EmptyValue(); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + /** * Deserializer. */ diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ExistsValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ExistsValue.java index 621c97877c..0fa64fe1b6 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ExistsValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ExistsValue.java @@ -88,7 +88,7 @@ public ValueWithChild withNewChild(@Nonnull final Value newChild) { public int hashCodeWithoutChildren() { return PlanHashable.objectsPlanHash(PlanHashable.CURRENT_FOR_CONTINUATION, BASE_HASH); } - + @Override public int planHash(@Nonnull final PlanHashMode mode) { return PlanHashable.objectsPlanHash(mode, BASE_HASH, child); @@ -138,6 +138,12 @@ public static ExistsValue fromProto(@Nonnull final PlanSerializationContext seri Objects.requireNonNull(existsValueProto.getChild()))); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(getChild()); + } + /** * A function that checks whether an item exists in a {@link RelationalExpression}. */ 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 ef35dee453..5ad2f380f1 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 @@ -185,7 +185,7 @@ public boolean equalsWithoutChildren(@Nonnull final Value other, @Nonnull final final var that = (FieldValue)other; return fieldPath.equals(that.fieldPath) && - childValue.semanticEquals(that.childValue, equivalenceMap); + childValue.semanticEquals(that.childValue, equivalenceMap); } @Override @@ -206,7 +206,7 @@ public boolean subsumedBy(@Nullable final Value other, @Nonnull final AliasMap e public int hashCodeWithoutChildren() { return PlanHashable.objectsPlanHash(PlanHashable.CURRENT_FOR_CONTINUATION, BASE_HASH, fieldPath); } - + @Override public int planHash(@Nonnull final PlanHashMode mode) { return PlanHashable.objectsPlanHash(mode, BASE_HASH, fieldPath); @@ -355,6 +355,12 @@ public static Optional stripFieldPrefixMaybe(@Nonnull FieldPath field return Optional.of(fieldPath.subList(potentialPrefixPath.size(), fieldPath.size())); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(getChild()); + } + /** * A list of fields forming a path. */ diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/InOpValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/InOpValue.java index 70ad7d9348..56f75be4ea 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/InOpValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/InOpValue.java @@ -80,11 +80,8 @@ private InOpValue(@Nonnull final Value probeValue, @Nonnull @Override - public Iterable getChildren() { - final var builder = new ImmutableList.Builder(); - builder.add(probeValue); - builder.add(inArrayValue); - return builder.build(); + protected Iterable computeChildren() { + return ImmutableList.of(probeValue, inArrayValue); } @Nonnull diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexOnlyAggregateValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexOnlyAggregateValue.java index b8552c378e..86550b52b9 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexOnlyAggregateValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexOnlyAggregateValue.java @@ -122,6 +122,12 @@ public Value getChild() { return child; } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(getChild()); + } + @Nonnull @Override public Type getResultType() { diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexedValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexedValue.java index a10a0b5281..97cdfeed3a 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexedValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexedValue.java @@ -31,6 +31,7 @@ import com.apple.foundationdb.record.query.plan.cascades.AliasMap; import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; import javax.annotation.Nonnull; import java.util.Objects; @@ -40,6 +41,7 @@ */ @API(API.Status.EXPERIMENTAL) public class IndexedValue extends AbstractValue implements LeafValue, Value.CompileTimeValue { + private static final ObjectPlanHash BASE_HASH = new ObjectPlanHash("Indexed-Value"); @Nonnull @@ -49,6 +51,12 @@ public IndexedValue() { this(Type.primitiveType(Type.TypeCode.UNKNOWN)); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + public IndexedValue(@Nonnull final Type resultType) { this.resultType = resultType; } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/LikeOperatorValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/LikeOperatorValue.java index 91ed72b838..ba53ab5b54 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/LikeOperatorValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/LikeOperatorValue.java @@ -108,7 +108,7 @@ public Optional toQueryPredicate(@Nullable final TypeRepository @Nonnull @Override - public Iterable getChildren() { + protected Iterable computeChildren() { return ImmutableList.of(srcChild, patternChild); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/LiteralValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/LiteralValue.java index 8844e80162..4abb29cf1c 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/LiteralValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/LiteralValue.java @@ -39,6 +39,7 @@ import com.google.auto.service.AutoService; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Verify; +import com.google.common.collect.ImmutableList; import com.google.protobuf.Message; import javax.annotation.Nonnull; @@ -248,6 +249,12 @@ public static LiteralValue> ofList(@Nonnull final List listValue) return new LiteralValue<>(new Type.Array(resolvedElementType), listValue); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + /** * Deserializer. * @param the type of literal diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/NullValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/NullValue.java index 1b1c35a230..c584a19894 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/NullValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/NullValue.java @@ -34,6 +34,7 @@ import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.google.auto.service.AutoService; import com.google.common.base.Verify; +import com.google.common.collect.ImmutableList; import com.google.protobuf.Message; import javax.annotation.Nonnull; @@ -155,6 +156,12 @@ public static NullValue fromProto(@Nonnull final PlanSerializationContext serial return new NullValue(Type.fromTypeProto(serializationContext, Objects.requireNonNull(nullValueProto.getResultType()))); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + /** * Deserializer. */ diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/NumericAggregationValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/NumericAggregationValue.java index 5e967ebc90..e9984277e0 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/NumericAggregationValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/NumericAggregationValue.java @@ -132,6 +132,12 @@ public Value getChild() { return child; } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(getChild()); + } + @Override public int hashCodeWithoutChildren() { return PlanHashable.objectsPlanHash(PlanHashable.CURRENT_FOR_CONTINUATION, BASE_HASH, operator); diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ObjectValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ObjectValue.java index 4cc80e776e..8f3f52fe9a 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ObjectValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ObjectValue.java @@ -36,6 +36,7 @@ import com.apple.foundationdb.record.query.plan.cascades.Formatter; import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.protobuf.Message; @@ -75,6 +76,12 @@ public Set getCorrelatedToWithoutChildren() { return ImmutableSet.of(alias); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + @Nonnull @Override public Value rebaseLeaf(@Nonnull final CorrelationIdentifier targetAlias) { diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/OfTypeValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/OfTypeValue.java index ceddceac5b..b0046babfc 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/OfTypeValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/OfTypeValue.java @@ -32,6 +32,7 @@ import com.apple.foundationdb.record.query.plan.cascades.AliasMap; import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; import com.google.protobuf.DynamicMessage; import com.google.protobuf.Message; @@ -158,6 +159,12 @@ public static OfTypeValue from(@Nonnull final ConstantObjectValue value) { return new OfTypeValue(ConstantObjectValue.of(value.getAlias(), value.getOrdinal(), Type.any()), value.getResultType()); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(getChild()); + } + /** * Deserializer. */ diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PatternForLikeValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PatternForLikeValue.java index 4eb50f5929..64da133634 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PatternForLikeValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PatternForLikeValue.java @@ -101,8 +101,8 @@ public String explain(@Nonnull final Formatter formatter) { @Nonnull @Override - public Iterable getChildren() { - return ImmutableList.of(patternChild, escapeChild); + protected Iterable computeChildren() { + return ImmutableList.of(patternChild, escapeChild); } @Nonnull diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PickValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PickValue.java index c77b8c25d8..807c1541f2 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PickValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PickValue.java @@ -87,7 +87,7 @@ private PickValue(@Nonnull final Value selectorValue, @Nonnull final Iterable getChildren() { + protected Iterable computeChildren() { return children; } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PromoteValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PromoteValue.java index cf10223829..274a8c1768 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PromoteValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/PromoteValue.java @@ -44,6 +44,7 @@ import com.google.common.base.Suppliers; import com.google.common.base.Verify; import com.google.common.collect.BiMap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.protobuf.Descriptors; import com.google.protobuf.Message; @@ -185,8 +186,8 @@ public PromoteValue(@Nonnull final Value inValue, @Nonnull final Type promoteToT this.promoteToType = promoteToType; this.promotionTrie = promotionTrie; this.isSimplePromotion = promoteToType.isPrimitive() || - (promoteToType instanceof Type.Array && - Objects.requireNonNull(((Type.Array)promoteToType).getElementType()).isPrimitive()); + (promoteToType instanceof Type.Array && + Objects.requireNonNull(((Type.Array)promoteToType).getElementType()).isPrimitive()); } @Nonnull @@ -227,11 +228,17 @@ public Type getResultType() { return promoteToType; } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(getChild()); + } + @Override public int hashCodeWithoutChildren() { return PlanHashable.objectsPlanHash(PlanHashable.CURRENT_FOR_CONTINUATION, BASE_HASH, promoteToType); } - + @Override public int planHash(@Nonnull final PlanHashMode mode) { return PlanHashable.objectsPlanHash(mode, BASE_HASH, inValue, promoteToType); diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/QuantifiedObjectValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/QuantifiedObjectValue.java index bf5d2d0147..effd57698d 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/QuantifiedObjectValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/QuantifiedObjectValue.java @@ -37,6 +37,7 @@ import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.apple.foundationdb.record.query.plan.plans.QueryResult; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.protobuf.Message; @@ -104,6 +105,12 @@ public Set getCorrelatedToWithoutChildren() { return QuantifiedValue.super.getCorrelatedToWithoutChildren(); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + @Nonnull @Override public String explain(@Nonnull final Formatter formatter) { diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/QueriedValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/QueriedValue.java index b8f9724800..5da3df3a81 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/QueriedValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/QueriedValue.java @@ -32,6 +32,7 @@ import com.apple.foundationdb.record.query.plan.cascades.Formatter; import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; import javax.annotation.Nonnull; import java.util.Objects; @@ -50,6 +51,12 @@ public QueriedValue() { this(Type.primitiveType(Type.TypeCode.UNKNOWN)); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + public QueriedValue(@Nonnull final Type resultType) { this.resultType = resultType; } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RecordConstructorValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RecordConstructorValue.java index 5d98d74b7a..be33ba3ee7 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RecordConstructorValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RecordConstructorValue.java @@ -74,14 +74,11 @@ public class RecordConstructorValue extends AbstractValue implements AggregateVa @Nonnull protected final List> columns; @Nonnull - private final Supplier> childrenSupplier; - @Nonnull private final Supplier hashCodeWithoutChildrenSupplier; private RecordConstructorValue(@Nonnull Collection> columns, @Nonnull final Type.Record resultType) { this.resultType = resultType; this.columns = ImmutableList.copyOf(columns); - this.childrenSupplier = Suppliers.memoize(this::computeChildren); this.hashCodeWithoutChildrenSupplier = Suppliers.memoize(this::computeHashCodeWithoutChildren); } @@ -92,11 +89,7 @@ public List> getColumns() { @Nonnull @Override - public Iterable getChildren() { - return childrenSupplier.get(); - } - - private List computeChildren() { + protected List computeChildren() { return columns .stream() .map(Column::getValue) diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RecordTypeValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RecordTypeValue.java index 7bb2cb3e24..14cf55effc 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RecordTypeValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RecordTypeValue.java @@ -36,6 +36,7 @@ import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.apple.foundationdb.record.query.plan.plans.QueryResult; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.protobuf.Message; @@ -57,7 +58,7 @@ public class RecordTypeValue extends AbstractValue implements QuantifiedValue { public RecordTypeValue(@Nonnull final CorrelationIdentifier alias) { this.alias = alias; } - + @Nonnull @Override public Value rebaseLeaf(@Nonnull final CorrelationIdentifier targetAlias) { @@ -91,6 +92,12 @@ public Set getCorrelatedToWithoutChildren() { return QuantifiedValue.super.getCorrelatedToWithoutChildren(); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + @Override public int hashCodeWithoutChildren() { return PlanHashable.objectPlanHash(PlanHashable.CURRENT_FOR_CONTINUATION, BASE_HASH); diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RelOpValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RelOpValue.java index fac9e96709..8e69307c03 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RelOpValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/RelOpValue.java @@ -120,7 +120,7 @@ protected RelOpValue(@Nonnull final String functionName, @Nonnull @Override - public Iterable getChildren() { + protected Iterable computeChildren() { return children; } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/UdfValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/UdfValue.java index a4d621d9e7..91bfd8444d 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/UdfValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/UdfValue.java @@ -65,7 +65,7 @@ public int planHash(@Nonnull final PlanHashMode hashKind) { @Nonnull @Override - public Iterable getChildren() { + protected Iterable computeChildren() { return children; } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/Value.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/Value.java index 406661ec64..5320b356f3 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/Value.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/Value.java @@ -74,7 +74,6 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Stream; -import java.util.stream.StreamSupport; /** * A scalar value type. @@ -144,8 +143,7 @@ default String explain(@Nonnull final Formatter formatter) { */ default boolean isConstant() { return getCorrelatedTo().isEmpty() - && StreamSupport.stream(filter(NondeterministicValue.class::isInstance).spliterator(), false) - .findAny().isEmpty(); // TODO: use CompileTime tag interface. + && preOrderStream().filter(NondeterministicValue.class::isInstance).findAny().isEmpty(); } /** @@ -250,8 +248,7 @@ default boolean isFunctionallyDependentOn(@Nonnull final Value otherValue) { return false; } - return StreamSupport.stream(inPreOrder().spliterator(), false) - .flatMap(value -> value instanceof QuantifiedValue ? Stream.of((QuantifiedValue)value) : Stream.empty()) + return preOrderStream().flatMap(value -> value instanceof QuantifiedValue ? Stream.of((QuantifiedValue)value) : Stream.empty()) .allMatch(quantifiedValue -> quantifiedValue.isFunctionallyDependentOn(otherValue)); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ValueWithChild.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ValueWithChild.java index 28054feb3d..c43dfcf9ba 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ValueWithChild.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/ValueWithChild.java @@ -21,11 +21,9 @@ package com.apple.foundationdb.record.query.plan.cascades.values; import com.apple.foundationdb.annotation.API; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import javax.annotation.Nonnull; -import java.util.List; /** * A scalar value type that has children. @@ -33,16 +31,6 @@ @API(API.Status.EXPERIMENTAL) public interface ValueWithChild extends Value { - /** - * Method to retrieve a list of children values. - * @return a list of children - */ - @Nonnull - @Override - default List getChildren() { - return ImmutableList.of(getChild()); - } - /** * Method to retrieve the only child value. * @return this child {@link Value} diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/VariadicFunctionValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/VariadicFunctionValue.java index f9f9332f3a..521c4164b8 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/VariadicFunctionValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/VariadicFunctionValue.java @@ -84,7 +84,7 @@ public class VariadicFunctionValue extends AbstractValue { public VariadicFunctionValue(@Nonnull PhysicalOperator operator, @Nonnull List children) { this.operator = operator; - this.children = children; + this.children = ImmutableList.copyOf(children); } @Nullable @@ -108,8 +108,8 @@ public Type getResultType() { @Nonnull @Override - public Iterable getChildren() { - return ImmutableList.copyOf(children); + protected Iterable computeChildren() { + return children; } @Nonnull diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/VersionValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/VersionValue.java index 81ba10dfc1..8e424152e9 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/VersionValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/VersionValue.java @@ -37,6 +37,7 @@ import com.apple.foundationdb.record.query.plan.cascades.typing.Type; import com.apple.foundationdb.record.query.plan.plans.QueryResult; import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; import com.google.protobuf.Message; import javax.annotation.Nonnull; @@ -84,6 +85,12 @@ public Set getCorrelatedToWithoutChildren() { return QuantifiedValue.super.getCorrelatedToWithoutChildren(); } + @Nonnull + @Override + protected Iterable computeChildren() { + return ImmutableList.of(); + } + @Nonnull @Override public String explain(@Nonnull final Formatter formatter) { diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/WindowedValue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/WindowedValue.java index a636b42d7b..273ab16c32 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/WindowedValue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/WindowedValue.java @@ -83,7 +83,7 @@ public List getArgumentValues() { @Nonnull @Override - public Iterable getChildren() { + protected Iterable computeChildren() { return ImmutableList.builder().addAll(partitioningValues).addAll(argumentValues).build(); } diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/util/TrieNode.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/util/TrieNode.java index 528f0da19e..e9629d01bf 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/util/TrieNode.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/util/TrieNode.java @@ -21,15 +21,16 @@ package com.apple.foundationdb.record.util; import com.apple.foundationdb.record.query.plan.cascades.TreeLike; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Streams; import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.Collection; import java.util.Map; import java.util.Objects; +import java.util.function.Supplier; import java.util.stream.Stream; /** @@ -44,10 +45,15 @@ public abstract class TrieNode> implements Tre private final T value; @Nullable private final Map childrenMap; + @Nonnull + private final Supplier> childrenSupplier = Suppliers.memoize(this::computeChildren); + @Nonnull + private final Supplier heightSupplier; public TrieNode(@Nullable final T value, @Nullable final Map childrenMap) { this.value = value; this.childrenMap = childrenMap == null ? null : ImmutableMap.copyOf(childrenMap); + this.heightSupplier = Suppliers.memoize(TreeLike.super::height); } @Nullable @@ -60,10 +66,20 @@ public Map getChildrenMap() { return childrenMap; } + @Nonnull + private Iterable computeChildren() { + return childrenMap == null ? ImmutableList.of() : childrenMap.values(); + } + @Nonnull @Override public Iterable getChildren() { - return childrenMap == null ? ImmutableList.of() : childrenMap.values(); + return childrenSupplier.get(); + } + + @Override + public int height() { + return heightSupplier.get(); } @Nonnull @@ -74,7 +90,7 @@ public N withChildren(final Iterable newChildren) { @Nonnull public Collection values() { - return Streams.stream(inPreOrder()) + return preOrderStream() .flatMap(trie -> trie.getValue() == null ? Stream.of() : Stream.of(trie.getValue())) .collect(ImmutableList.toImmutableList()); } diff --git a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/BooleanValueTest.java b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/BooleanValueTest.java index ff4ccb5080..a15e0dc21f 100644 --- a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/BooleanValueTest.java +++ b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/BooleanValueTest.java @@ -116,9 +116,10 @@ public int hashCodeWithoutChildren() { return 0; } - @Nonnull + @SuppressWarnings("NullableProblems") // this is for testing. + @Nullable @Override - public Iterable getChildren() { + protected Iterable computeChildren() { return null; } diff --git a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/TreeLikeTest.java b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/TreeLikeTest.java index ff78ca817e..9319264733 100644 --- a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/TreeLikeTest.java +++ b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/TreeLikeTest.java @@ -39,15 +39,18 @@ /** * Tests for {@link TreeLike}. */ +@SuppressWarnings("deprecation") // this is due to testing the deprecated {@code TreeLike#inPreOrder} for correctness. public class TreeLikeTest { @Test void testPreOrder1() { final TreeNode t = node("a", node("b"), node("c")); - final ImmutableList traversed = ImmutableList.copyOf(t.inPreOrder()); + final ImmutableList traversed = ImmutableList.copyOf(t.preOrderIterable()); assertEquals(ImmutableList.of("a", "b", "c"), traversed.stream().map(TreeNode::getContents).collect(ImmutableList.toImmutableList())); + assertEquals(ImmutableList.of("a", "b", "c"), + t.preOrderStream().map(item -> item.contents).collect(Collectors.toList())); } @Test @@ -59,25 +62,63 @@ void testPreOrder2() { node("d"), node("e"))); - final ImmutableList traversed = ImmutableList.copyOf(t.inPreOrder()); + final ImmutableList traversed = ImmutableList.copyOf(t.preOrderIterable()); + assertEquals(ImmutableList.of("a", "b", "c", "d", "e"), + traversed.stream().map(TreeNode::getContents).collect(ImmutableList.toImmutableList())); assertEquals(ImmutableList.of("a", "b", "c", "d", "e"), + t.preOrderStream().map(item -> item.contents).collect(Collectors.toList())); + } + + @Test + void testPreOrder3() { + final TreeNode t = + node("a", + node("b", + node("c"), + node("d")), + node("e", + node("f", + node("g"), + node("h")), + node("i"))); + + final ImmutableList traversed = ImmutableList.copyOf(t.preOrderIterable()); + assertEquals(ImmutableList.of("a", "b", "c", "d", "e", "f", "g", "h", "i"), + traversed.stream().map(TreeNode::getContents).collect(ImmutableList.toImmutableList())); + assertEquals(ImmutableList.of("a", "b", "c", "d", "e", "f", "g", "h", "i"), + t.preOrderStream().map(item -> item.contents).collect(Collectors.toList())); + } + + @Test + void testPreOrder4() { + final TreeNode t = + node("a", + node("b", + node("c"))); + + final ImmutableList traversed = ImmutableList.copyOf(t.preOrderIterable()); + assertEquals(ImmutableList.of("a", "b", "c"), traversed.stream().map(TreeNode::getContents).collect(ImmutableList.toImmutableList())); + assertEquals(ImmutableList.of("a", "b", "c"), + t.preOrderStream().map(item -> item.contents).collect(Collectors.toList())); } @Test void testPreOrderSingle() { final TreeNode t = node("a"); - final ImmutableList traversed = ImmutableList.copyOf(t.inPreOrder()); + final ImmutableList traversed = ImmutableList.copyOf(t.preOrderIterable()); assertEquals(ImmutableList.of("a"), traversed.stream().map(TreeNode::getContents).collect(ImmutableList.toImmutableList())); + assertEquals(ImmutableList.of("a"), + t.preOrderStream().map(item -> item.contents).collect(Collectors.toList())); } @Test void testPostOrder1() { final TreeNode t = node("a", node("b"), node("c")); - final ImmutableList traversed = ImmutableList.copyOf(t.inPostOrder()); + final ImmutableList traversed = ImmutableList.copyOf(t.postOrderIterable()); assertEquals(ImmutableList.of("b", "c", "a"), traversed.stream().map(TreeNode::getContents).collect(ImmutableList.toImmutableList())); } @@ -91,7 +132,7 @@ void testPostOrder2() { node("d"), node("e"))); - final ImmutableList traversed = ImmutableList.copyOf(t.inPostOrder()); + final ImmutableList traversed = ImmutableList.copyOf(t.postOrderIterable()); assertEquals(ImmutableList.of("b", "d", "e", "c", "a"), traversed.stream().map(TreeNode::getContents).collect(ImmutableList.toImmutableList())); } @@ -100,7 +141,7 @@ void testPostOrder2() { void testPostOrderSingle() { final TreeNode t = node("a"); - final ImmutableList traversed = ImmutableList.copyOf(t.inPostOrder()); + final ImmutableList traversed = ImmutableList.copyOf(t.postOrderIterable()); assertEquals(ImmutableList.of("a"), traversed.stream().map(TreeNode::getContents).collect(ImmutableList.toImmutableList())); } diff --git a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/values/ValueHeightTest.java b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/values/ValueHeightTest.java new file mode 100644 index 0000000000..fefd2c0397 --- /dev/null +++ b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/values/ValueHeightTest.java @@ -0,0 +1,65 @@ +/* + * ValueHeightTest.java + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2015-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. + */ + +package com.apple.foundationdb.record.query.plan.cascades.values; + +import com.google.common.collect.ImmutableList; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import javax.annotation.Nonnull; +import java.util.Random; + +/** + * This tests the calculation of {@link Value#height()}. + */ +public class ValueHeightTest { + + @Nonnull + private static final Random random = new Random(); + + @Nonnull + private static Value valueOfDepth(int depth) { + assert depth > 0; + if (depth == 1) { + return LiteralValue.ofScalar(random.nextInt(1000)); + } + int childrenCount = random.nextInt(5) + 1; + int branchingNodeIndex = random.nextInt(childrenCount); + final ImmutableList.Builder childrenValuesBuilder = ImmutableList.builder(); + for (int i = 0; i < childrenCount; i++) { + if (i == branchingNodeIndex) { + childrenValuesBuilder.add(valueOfDepth(depth - 1)); + } else { + childrenValuesBuilder.add(LiteralValue.ofScalar(random.nextInt(1000))); + } + } + return RecordConstructorValue.ofUnnamed(childrenValuesBuilder.build()); + } + + @Test + void valueHeightIsCalculatedCorrectly() { + Assertions.assertEquals(1, valueOfDepth(1).height()); + for (int i = 0; i < 10000; i++) { + final int depth = random.nextInt(100) + 1; + Assertions.assertEquals(depth, valueOfDepth(depth).height()); + } + } +}