Skip to content

Commit 2bdc94d

Browse files
committed
Avoid breaking correlations when rewriting the logical query graph.
1 parent 54ebe84 commit 2bdc94d

File tree

7 files changed

+58
-46
lines changed

7 files changed

+58
-46
lines changed

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/translation/ToUniqueAliasesTranslationMap.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,14 @@
2525
import com.apple.foundationdb.record.query.plan.cascades.Quantifier;
2626
import com.apple.foundationdb.record.query.plan.cascades.values.LeafValue;
2727
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
28+
import com.google.common.base.Verify;
2829

2930
import javax.annotation.Nonnull;
3031
import javax.annotation.Nullable;
3132
import java.util.LinkedHashMap;
3233
import java.util.Map;
3334
import java.util.Optional;
35+
import java.util.Set;
3436

3537
/**
3638
* Translation map that allows for rebasing of graphs in a way that the resulting rebased graph is only using unique
@@ -43,8 +45,10 @@ public class ToUniqueAliasesTranslationMap implements TranslationMap {
4345
@Nonnull
4446
private final Map<CorrelationIdentifier, CorrelationIdentifier> sourceToTargetMap;
4547

46-
public ToUniqueAliasesTranslationMap() {
48+
private ToUniqueAliasesTranslationMap(@Nonnull final AliasMap identities) {
49+
Verify.verify(identities.definesOnlyIdentities());
4750
this.sourceToTargetMap = new LinkedHashMap<>();
51+
identities.forEachMapping(sourceToTargetMap::put);
4852
}
4953

5054
@Nonnull
@@ -87,4 +91,14 @@ private CorrelationIdentifier computeTargetIfAbsent(@Nonnull final CorrelationId
8791
return sourceToTargetMap.computeIfAbsent(sourceAlias,
8892
ignored0 -> Quantifier.uniqueId());
8993
}
94+
95+
@Nonnull
96+
public static ToUniqueAliasesTranslationMap newInstance() {
97+
return new ToUniqueAliasesTranslationMap(AliasMap.emptyMap());
98+
}
99+
100+
@Nonnull
101+
public static ToUniqueAliasesTranslationMap newInstance(@Nonnull final Set<CorrelationIdentifier> constantAliases) {
102+
return new ToUniqueAliasesTranslationMap(AliasMap.identitiesFor(constantAliases));
103+
}
90104
}

fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/RuleTestHelper.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ private TestRuleExecution run(RelationalExpression original, EvaluationContext e
149149
// rewriting and planning
150150
final var copiedOriginal =
151151
Iterables.getOnlyElement(References.rebaseGraphs(ImmutableList.of(Reference.initialOf(original)),
152-
Memoizer.noMemoization(PlannerStage.INITIAL), new ToUniqueAliasesTranslationMap(), false)).get();
152+
Memoizer.noMemoization(PlannerStage.INITIAL), ToUniqueAliasesTranslationMap.newInstance(), false)).get();
153153
ensureStage(PlannerStage.CANONICAL, copiedOriginal);
154154
if (rule instanceof ImplementationCascadesRule) {
155155
//
@@ -245,7 +245,7 @@ public TestRuleExecution assertYields(RelationalExpression original, EvaluationC
245245
//
246246
final var copiedExpected =
247247
Iterables.getOnlyElement(References.rebaseGraphs(ImmutableList.of(Reference.initialOf(expression)),
248-
Memoizer.noMemoization(PlannerStage.INITIAL), new ToUniqueAliasesTranslationMap(), false)).get();
248+
Memoizer.noMemoization(PlannerStage.INITIAL), ToUniqueAliasesTranslationMap.newInstance(), false)).get();
249249
ensureStage(PlannerStage.CANONICAL, copiedExpected);
250250
expectedListBuilder.add(copiedExpected);
251251
}

fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/query/plan/cascades/TranslateGraphTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ void rebaseGraphTest() {
9292
final var qun = Quantifier.forEach(Reference.initialOf(graphExpansionBuilder.build().buildSelect()));
9393
final var expression = LogicalSortExpression.unsorted(qun);
9494
final var reference = Reference.initialOf(expression);
95-
final var translationMap = new ToUniqueAliasesTranslationMap();
95+
final var translationMap = ToUniqueAliasesTranslationMap.newInstance();
9696
final var translatedReferences =
9797
References.rebaseGraphs(ImmutableList.of(reference),
9898
Memoizer.noMemoization(PlannerStage.INITIAL),
@@ -159,7 +159,7 @@ void rebaseGraphTestWithDiamond() {
159159
final var qun = Quantifier.forEach(Reference.initialOf(graphExpansionBuilder.build().buildSelect()));
160160
final var expression = LogicalSortExpression.unsorted(qun);
161161
final var reference = Reference.initialOf(expression);
162-
final var translationMap = new ToUniqueAliasesTranslationMap();
162+
final var translationMap = ToUniqueAliasesTranslationMap.newInstance();
163163
final var translatedReferences =
164164
References.rebaseGraphs(ImmutableList.of(reference),
165165
Memoizer.noMemoization(PlannerStage.INITIAL),

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/metadata/RecordLayerInvokedRoutine.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
import com.apple.foundationdb.record.query.plan.cascades.RawSqlFunction;
2424
import com.apple.foundationdb.relational.api.metadata.InvokedRoutine;
2525
import com.apple.foundationdb.relational.recordlayer.query.functions.CompiledSqlFunction;
26+
import com.apple.foundationdb.relational.recordlayer.util.MemoizedFunction;
2627
import com.apple.foundationdb.relational.util.Assert;
27-
import com.google.common.base.Suppliers;
2828

2929
import javax.annotation.Nonnull;
3030
import java.util.Objects;
@@ -55,10 +55,7 @@ public RecordLayerInvokedRoutine(@Nonnull final String description,
5555
this.normalizedDescription = normalizedDescription;
5656
this.name = name;
5757
this.isTemporary = isTemporary;
58-
// TODO this used to be memoized
59-
final var memoizedTrue = Suppliers.memoize(() -> compilableSqlFunctionSupplier.apply(true));
60-
final var memoizedFalse = Suppliers.memoize(() -> compilableSqlFunctionSupplier.apply(false));
61-
this.compilableSqlFunctionSupplier = param -> param ? memoizedTrue.get() : memoizedFalse.get();
58+
this.compilableSqlFunctionSupplier = MemoizedFunction.memoize(compilableSqlFunctionSupplier::apply);
6259
}
6360

6461
@Nonnull

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/SemanticAnalyzer.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -842,19 +842,20 @@ public LogicalOperator resolveTableFunction(@Nonnull final Identifier functionNa
842842
final var resultingValue = arguments.allNamedArguments()
843843
? tableFunction.encapsulate(arguments.toNamedArgumentInvocation())
844844
: tableFunction.encapsulate(valueArgs);
845+
final var correlations = ((Correlated<?>)Assert.castUnchecked(resultingValue, Correlated.class)).getCorrelatedTo();
845846
if (resultingValue instanceof StreamingValue) {
846847
final var tableFunctionExpression = new TableFunctionExpression(Assert.castUnchecked(resultingValue, StreamingValue.class));
847848
final var reference = Reference.initialOf(tableFunctionExpression);
848849
final var translatedReference = Iterables.getOnlyElement(References.rebaseGraphs(List.of(reference),
849-
Memoizer.noMemoization(PlannerStage.INITIAL), new ToUniqueAliasesTranslationMap(), false));
850+
Memoizer.noMemoization(PlannerStage.INITIAL), ToUniqueAliasesTranslationMap.newInstance(correlations), false));
850851
final var resultingQuantifier = Quantifier.forEach(translatedReference);
851852
final var output = Expressions.of(LogicalOperator.convertToExpressions(resultingQuantifier));
852853
return LogicalOperator.newNamedOperator(functionName, output, resultingQuantifier);
853854
}
854855
final var relationalExpression = Assert.castUnchecked(resultingValue, RelationalExpression.class);
855856
final var reference = Reference.initialOf(relationalExpression);
856857
final var translatedReference = Iterables.getOnlyElement(References.rebaseGraphs(List.of(reference),
857-
Memoizer.noMemoization(PlannerStage.INITIAL), new ToUniqueAliasesTranslationMap(), false));
858+
Memoizer.noMemoization(PlannerStage.INITIAL), ToUniqueAliasesTranslationMap.newInstance(correlations), false));
858859
final var topQun = Quantifier.forEach(translatedReference);
859860
return LogicalOperator.newNamedOperator(functionName, Expressions.fromQuantifier(topQun), topQun);
860861
}

0 commit comments

Comments
 (0)