Skip to content

Reactivate amortization of plan generation of functions. #3595

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Reactivate amortization of plan generation of functions. #3595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bafb313
reactivate amortization of plan generation of functions.
hatyo Sep 12, 2025
8575578
remove debugging code.
hatyo Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...ava/com/apple/foundationdb/relational/recordlayer/metadata/RecordLayerInvokedRoutine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.apple.foundationdb.relational.api.metadata.InvokedRoutine;
import com.apple.foundationdb.relational.recordlayer.query.functions.CompiledSqlFunction;
import com.apple.foundationdb.relational.util.Assert;
import com.google.common.base.Suppliers;

import javax.annotation.Nonnull;
import java.util.Objects;
Expand Down Expand Up @@ -55,7 +56,9 @@ public RecordLayerInvokedRoutine(@Nonnull final String description,
this.name = name;
this.isTemporary = isTemporary;
// TODO this used to be memoized
this.compilableSqlFunctionSupplier = compilableSqlFunctionSupplier;
final var memoizedTrue = Suppliers.memoize(() -> compilableSqlFunctionSupplier.apply(true));
final var memoizedFalse = Suppliers.memoize(() -> compilableSqlFunctionSupplier.apply(false));
this.compilableSqlFunctionSupplier = param -> param ? memoizedTrue.get() : memoizedFalse.get();
}

@Nonnull
Expand Down
15 changes: 13 additions & 2 deletions ...e/src/main/java/com/apple/foundationdb/relational/recordlayer/query/SemanticAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@
import com.apple.foundationdb.record.query.plan.cascades.Correlated;
import com.apple.foundationdb.record.query.plan.cascades.CorrelationIdentifier;
import com.apple.foundationdb.record.query.plan.cascades.IndexAccessHint;
import com.apple.foundationdb.record.query.plan.cascades.Memoizer;
import com.apple.foundationdb.record.query.plan.cascades.PlannerStage;
import com.apple.foundationdb.record.query.plan.cascades.Quantifier;
import com.apple.foundationdb.record.query.plan.cascades.Reference;
import com.apple.foundationdb.record.query.plan.cascades.References;
import com.apple.foundationdb.record.query.plan.cascades.expressions.RelationalExpression;
import com.apple.foundationdb.record.query.plan.cascades.expressions.TableFunctionExpression;
import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
Expand All @@ -48,6 +51,7 @@
import com.apple.foundationdb.record.query.plan.cascades.values.StreamableAggregateValue;
import com.apple.foundationdb.record.query.plan.cascades.values.StreamingValue;
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
import com.apple.foundationdb.record.query.plan.cascades.values.translation.ToUniqueAliasesTranslationMap;
import com.apple.foundationdb.record.util.pair.NonnullPair;
import com.apple.foundationdb.relational.api.exceptions.ErrorCode;
import com.apple.foundationdb.relational.api.exceptions.RelationalException;
Expand All @@ -66,6 +70,7 @@
import com.google.common.base.Functions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.google.protobuf.ByteString;
Expand Down Expand Up @@ -839,12 +844,18 @@ public LogicalOperator resolveTableFunction(@Nonnull final Identifier functionNa
: tableFunction.encapsulate(valueArgs);
if (resultingValue instanceof StreamingValue) {
final var tableFunctionExpression = new TableFunctionExpression(Assert.castUnchecked(resultingValue, StreamingValue.class));
final var resultingQuantifier = Quantifier.forEach(Reference.initialOf(tableFunctionExpression));
final var reference = Reference.initialOf(tableFunctionExpression);
final var translatedReference = Iterables.getOnlyElement(References.rebaseGraphs(List.of(reference),
Memoizer.noMemoization(PlannerStage.INITIAL), new ToUniqueAliasesTranslationMap(), false));
final var resultingQuantifier = Quantifier.forEach(translatedReference);
final var output = Expressions.of(LogicalOperator.convertToExpressions(resultingQuantifier));
return LogicalOperator.newNamedOperator(functionName, output, resultingQuantifier);
}
final var relationalExpression = Assert.castUnchecked(resultingValue, RelationalExpression.class);
final var topQun = Quantifier.forEach(Reference.initialOf(relationalExpression));
final var reference = Reference.initialOf(relationalExpression);
final var translatedReference = Iterables.getOnlyElement(References.rebaseGraphs(List.of(reference),
Memoizer.noMemoization(PlannerStage.INITIAL), new ToUniqueAliasesTranslationMap(), false));
final var topQun = Quantifier.forEach(translatedReference);
return LogicalOperator.newNamedOperator(functionName, Expressions.fromQuantifier(topQun), topQun);
}

Expand Down
53 changes: 27 additions & 26 deletions ...java/com/apple/foundationdb/relational/recordlayer/metadata/SchemaTemplateSerDeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand All @@ -61,6 +60,7 @@

import javax.annotation.Nonnull;
import java.net.URI;
import java.sql.SQLException;
import java.util.BitSet;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -362,11 +362,10 @@ public void findTableByNameWorksCorrectly() {
Assertions.assertFalse(nonExisting.isPresent());
}

@Disabled
@Test
public void sqlFunctionsAreLazilyParsed() throws RelationalException {
public void sqlFunctionsAreLazilyParsed() throws Exception {
final var peekingDeserializer = recMetadataSampleWithFunctions(
"CREATE FUNCTION SqlFunction1(IN Q BIGINT) AS SELECT * FROM T1 WHERE col1 < Q");
"CREATE FUNCTION SqlFunction1(IN Q BIGINT) AS SELECT * FROM T1 WHERE COL1 < Q");
Assertions.assertTrue(peekingDeserializer.hasNoCompilationRequestsFor("SqlFunction1"));

final var planGenerator = peekingDeserializer.getPlanGenerator();
Expand All @@ -378,12 +377,11 @@ public void sqlFunctionsAreLazilyParsed() throws RelationalException {
Assertions.assertNotNull(plan);
}

@Disabled
@Test
public void nestedSqlFunctionsAreLazilyParsed() throws RelationalException {
public void nestedSqlFunctionsAreLazilyParsed() throws Exception {
final var peekingDeserializer = recMetadataSampleWithFunctions(
"CREATE FUNCTION SqlFunction1(IN Q BIGINT) AS SELECT * FROM T1 WHERE col1 < Q",
"CREATE FUNCTION SqlFunction2(IN Q BIGINT) AS SELECT * FROM SqlFunction1(100) WHERE col1 < Q");
"CREATE FUNCTION SqlFunction1(IN Q BIGINT) AS SELECT * FROM T1 WHERE COL1 < Q",
"CREATE FUNCTION SqlFunction2(IN Q BIGINT) AS SELECT * FROM SqlFunction1(100) WHERE COL1 < Q");
Assertions.assertTrue(peekingDeserializer.hasNoCompilationRequestsFor("SqlFunction1"));
Assertions.assertTrue(peekingDeserializer.hasNoCompilationRequestsFor("SqlFunction2"));

Expand All @@ -396,17 +394,16 @@ public void nestedSqlFunctionsAreLazilyParsed() throws RelationalException {
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction1"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction2"));

Assertions.assertDoesNotThrow(() -> planGenerator.getPlan("select * from SqlFunction2(200) where col1 < 300"));
Assertions.assertDoesNotThrow(() -> planGenerator.getPlan("select * from SqlFunction2(200) where COL1 < 300"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction1"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction2"));
}

@Disabled
@Test
public void onlyQueriedSqlFunctionsAreCompiled() throws RelationalException {
public void onlyQueriedSqlFunctionsAreCompiled() throws Exception {
final var peekingDeserializer = recMetadataSampleWithFunctions(
"CREATE FUNCTION SqlFunction1(IN Q BIGINT) AS SELECT * FROM T1 WHERE col1 < Q",
"CREATE FUNCTION SqlFunction2(IN Q BIGINT) AS SELECT * FROM SqlFunction1(100) WHERE col1 < Q",
"CREATE FUNCTION SqlFunction1(IN Q BIGINT) AS SELECT * FROM T1 WHERE COL1 < Q",
"CREATE FUNCTION SqlFunction2(IN Q BIGINT) AS SELECT * FROM SqlFunction1(100) WHERE COL1 < Q",
"CREATE FUNCTION SqlFunction3() AS SELECT * FROM T1");
Assertions.assertTrue(peekingDeserializer.hasNoCompilationRequestsFor("SqlFunction1"));
Assertions.assertTrue(peekingDeserializer.hasNoCompilationRequestsFor("SqlFunction2"));
Expand All @@ -418,17 +415,19 @@ public void onlyQueriedSqlFunctionsAreCompiled() throws RelationalException {
Assertions.assertTrue(peekingDeserializer.hasNoCompilationRequestsFor("SqlFunction2"));
Assertions.assertTrue(peekingDeserializer.hasNoCompilationRequestsFor("SqlFunction3"));

planGenerator.getPlan("select * from SqlFunction2(200)");

Assertions.assertDoesNotThrow(() -> planGenerator.getPlan("select * from SqlFunction2(200)"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction1"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction2"));
Assertions.assertTrue(peekingDeserializer.hasNoCompilationRequestsFor("SqlFunction3"));

Assertions.assertDoesNotThrow(() -> planGenerator.getPlan("select * from SqlFunction2(200) where col1 < 300"));
Assertions.assertDoesNotThrow(() -> planGenerator.getPlan("select * from SqlFunction2(200) where COL1 < 300"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction1"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction2"));
Assertions.assertTrue(peekingDeserializer.hasNoCompilationRequestsFor("SqlFunction4"));

Assertions.assertDoesNotThrow(() -> planGenerator.getPlan("select * from SqlFunction3() where col1 < 300"));
Assertions.assertDoesNotThrow(() -> planGenerator.getPlan("select * from SqlFunction3() where COL1 < 300"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction1"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction2"));
Assertions.assertTrue(peekingDeserializer.hasOneCompilationRequestFor("SqlFunction3"));
Expand Down Expand Up @@ -526,14 +525,16 @@ private static RecordMetadataDeserializerWithPeekingFunctionSupplier recMetadata

// Verify that the provided functions match the ones we just deserialized
Assertions.assertEquals(expectedFunctionMap, actualFunctionMap);

Assertions.assertTrue(invokedRoutines.containsKey("SqlFunction1"));
final var function = invokedRoutines.get("SqlFunction1");
Assertions.assertInstanceOf(RawSqlFunction.class, function);
final var rawSqlFunction = (RawSqlFunction)function;
Assertions.assertEquals("SqlFunction1", rawSqlFunction.getFunctionName());
Assertions.assertEquals("CREATE FUNCTION SqlFunction1(IN Q BIGINT) AS SELECT * FROM T1 WHERE col1 < Q",
rawSqlFunction.getDefinition());
for (final var entry : expectedFunctionMap.entrySet()) {
final var functionName = entry.getKey();
final var functionDescription = entry.getValue();
Assertions.assertTrue(invokedRoutines.containsKey(functionName));
final var function = invokedRoutines.get(functionName);
Assertions.assertInstanceOf(RawSqlFunction.class, function);
final var rawSqlFunction = (RawSqlFunction)function;
Assertions.assertEquals(functionName, rawSqlFunction.getFunctionName());
Assertions.assertEquals(functionDescription, rawSqlFunction.getDefinition());
}

// let's verify now that _no_ compilation is invoked when deserializing the record metadata.
// for that, we use a deserializer with peeking supplier to the function compilation logic.
Expand Down Expand Up @@ -586,8 +587,7 @@ boolean hasOneCompilationRequestFor(@Nonnull final String functionName) {
}

@Nonnull
public PlanGenerator getPlanGenerator()
throws RelationalException {
public PlanGenerator getPlanGenerator() throws RelationalException, SQLException {

final var metricCollector = new MetricCollector() {
@Override
Expand All @@ -610,7 +610,8 @@ public <T> T clock(@Nonnull RelationalMetric.RelationalEvent event,
.withPlannerConfiguration(PlannerConfiguration.ofAllAvailableIndexes())
.withUserVersion(0)
.build();
return PlanGenerator.create(Optional.empty(), ctx, ctx.getMetaData(), new RecordStoreState(null, Map.of()), Options.NONE);
return PlanGenerator.create(Optional.empty(), ctx, ctx.getMetaData(), new RecordStoreState(null, Map.of()),
Options.builder().withOption(Options.Name.CASE_SENSITIVE_IDENTIFIERS, true).build());
}
}
}
Loading