Skip to content

Commit 7119e7e

Browse files
g31pranjalg31pranjal
authored
remove CONTINUATION_CONTAINS_COMPILED_STATEMENT usage (#3650)
This PR does a couple of things: - removes the usage of `CONTINUATION_CONTAINS_COMPILED_STATEMENT` option in Connection and as a query option. - The option was, by default, set to true and is now true for all the requests to the engine. - We still support `SELECT ... WITH CONTINUATION ...`, which could be removed separately after this work. Hence, it could happen that a query started with a non-serialized continuation, but has now on serialised continuation. - JDBC driver still supports the option, for backward compatilibility. However, the current change always sets that to `TRUE` and rejects connections that have it set to `FALSE`. Once this is rolled out, we should probably stop failing on false and mark the option as deprecated in proto. Finally, we can stop setting the option and remove the option altogether. - Removes a few tests around testing this option with False.
1 parent 97b61cb commit 7119e7e

File tree

12 files changed

+10
-112
lines changed

12 files changed

+10
-112
lines changed

‎fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/Options.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,6 @@ public enum Name {
214214
*/
215215
VALID_PLAN_HASH_MODES,
216216

217-
/**
218-
* Boolean indicator if continuations generated for query responses may contain serialized compiled statements
219-
* that can be used in EXECUTE CONTINUATION statements.
220-
*/
221-
CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS,
222-
223217
/**
224218
* Timeout for asynchronous operations in milliseconds, this is usually used to set an upperbound time limit for
225219
* operations interacting with FDB.
@@ -294,7 +288,6 @@ public enum IndexFetchMethod {
294288
builder.put(Name.EXECUTION_SCANNED_ROWS_LIMIT, Integer.MAX_VALUE);
295289
builder.put(Name.DRY_RUN, false);
296290
builder.put(Name.CASE_SENSITIVE_IDENTIFIERS, false);
297-
builder.put(Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, true);
298291
builder.put(Name.ASYNC_OPERATIONS_TIMEOUT_MILLIS, 10_000L);
299292
builder.put(Name.ENCRYPT_WHEN_SERIALIZING, false);
300293
builder.put(Name.ENCRYPTION_KEY_PASSWORD, "");
@@ -551,7 +544,6 @@ private static Map<Name, List<OptionContract>> makeContracts() {
551544
data.put(Name.CASE_SENSITIVE_IDENTIFIERS, List.of(TypeContract.booleanType()));
552545
data.put(Name.CURRENT_PLAN_HASH_MODE, List.of(TypeContract.stringType()));
553546
data.put(Name.VALID_PLAN_HASH_MODES, List.of(TypeContract.stringType()));
554-
data.put(Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, List.of(TypeContract.booleanType()));
555547
data.put(Name.ASYNC_OPERATIONS_TIMEOUT_MILLIS, List.of(TypeContract.longType(), RangeContract.of(0L, Long.MAX_VALUE)));
556548
data.put(Name.ENCRYPT_WHEN_SERIALIZING, List.of(TypeContract.booleanType()));
557549
data.put(Name.ENCRYPTION_KEY_STORE, List.of(TypeContract.nullableStringType()));

‎fdb-relational-api/src/testFixtures/java/com/apple/foundationdb/relational/utils/OptionsTestHelper.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ public static Options nonDefaultOptions() throws SQLException {
5757
builder = builder.withOption(Options.Name.CASE_SENSITIVE_IDENTIFIERS, true);
5858
builder = builder.withOption(Options.Name.CURRENT_PLAN_HASH_MODE, "m1");
5959
builder = builder.withOption(Options.Name.VALID_PLAN_HASH_MODES, "m1,m2");
60-
builder = builder.withOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, false);
6160
builder = builder.withOption(Options.Name.ASYNC_OPERATIONS_TIMEOUT_MILLIS, 5000L);
6261
builder = builder.withOption(Options.Name.ENCRYPT_WHEN_SERIALIZING, true);
6362
builder = builder.withOption(Options.Name.ENCRYPTION_KEY_STORE, "secrets.ks");

‎fdb-relational-core/src/main/antlr/RelationalParser.g4‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,6 @@ queryOption
517517
: NOCACHE
518518
| LOG QUERY
519519
| DRY RUN
520-
| CONTINUATION CONTAINS COMPILED STATEMENT
521520
;
522521

523522
// Transaction's Statements

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,9 +301,6 @@ public Object visitQueryOption(@Nonnull RelationalParser.QueryOptionContext ctx)
301301
if (ctx.DRY() != null) {
302302
queryOptions.withOption(Options.Name.DRY_RUN, true);
303303
}
304-
if (ctx.CONTINUATION() != null) {
305-
queryOptions.withOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, true);
306-
}
307304
return null;
308305
} catch (SQLException e) {
309306
throw ExceptionUtil.toRelationalException(e).toUncheckedWrappedException();

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030

3131
import javax.annotation.Nonnull;
3232
import java.util.Arrays;
33-
import java.util.Objects;
3433
import java.util.Set;
3534

3635
@API(API.Status.INTERNAL)
@@ -56,10 +55,6 @@ public static Set<PlanHashable.PlanHashMode> getValidPlanHashModes(@Nonnull fina
5655
.collect(ImmutableSet.toImmutableSet());
5756
}
5857

59-
public static boolean getContinuationsContainsCompiledStatements(@Nonnull final Options options) {
60-
return Objects.requireNonNull(options.getOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS));
61-
}
62-
6358
@Nonnull
6459
public static IndexFetchMethod getIndexFetchMethod(@Nonnull final Options options) {
6560
Options.IndexFetchMethod indexFetchMethod = options.getOption(Options.Name.INDEX_FETCH_METHOD);

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,7 @@ protected <M extends Message> void validatePlanAgainstEnvironment(@Nonnull final
268268
PlanValidator.validateHashes(parsedContinuation, executionContext.metricCollector,
269269
recordQueryPlan, queryExecutionContext, currentPlanHashMode, validPlanHashModes);
270270

271-
final var options = executionContext.getOptions();
272-
final var continuationsContainCompiledStatements = OptionsUtils.getContinuationsContainsCompiledStatements(options);
273-
if (continuationsContainCompiledStatements && !parsedContinuation.atBeginning()) {
271+
if (!parsedContinuation.atBeginning()) {
274272
// if we are here it means that the current execution is from a regular planned plan, i.e. not
275273
// an EXECUTE CONTINUATION statement but it uses a continuation that is not at the beginning.
276274
// this can only happen if the query was started without the use of serialized plans, and we are now
@@ -411,21 +409,20 @@ private RelationalResultSet executePhysicalPlan(@Nonnull final RecordLayerSchema
411409
final ResumableIterator<Row> iterator = RecordLayerIterator.create(cursor, messageFDBQueriedRecord -> new MessageTuple(messageFDBQueriedRecord.getMessage()));
412410
return new RecordLayerResultSet(RelationalStructMetaData.of(dataType), iterator, connection,
413411
(continuation, reason) -> enrichContinuation(continuation,
414-
currentPlanHashMode, reason, OptionsUtils.getContinuationsContainsCompiledStatements(options)));
412+
currentPlanHashMode, reason));
415413
});
416414
}
417415

418416
@Nonnull
419417
private Continuation enrichContinuation(@Nonnull final Continuation continuation,
420418
@Nonnull final PlanHashMode currentPlanHashMode,
421-
@Nonnull final Continuation.Reason reason,
422-
final boolean serializeCompiledStatement) throws RelationalException {
419+
@Nonnull final Continuation.Reason reason) throws RelationalException {
423420
final var continuationBuilder = ContinuationImpl.copyOf(continuation).asBuilder()
424421
.withBindingHash(queryExecutionContext.getParameterHash())
425422
.withPlanHash(planHashSupplier.get())
426423
.withReason(reason);
427424
// Do not send the serialized plan unless we can continue with this continuation.
428-
if (serializeCompiledStatement && !continuation.atEnd()) {
425+
if (!continuation.atEnd()) {
429426
//
430427
// Note that serialization and deserialization of the constituent elements have to done in the same order
431428
// in order for the dictionary compression for type serialization to work properly. The order is

‎fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/JoinWithLimitTest.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,8 @@
2121
package com.apple.foundationdb.relational.recordlayer;
2222

2323
import com.apple.foundationdb.relational.api.Continuation;
24-
import com.apple.foundationdb.relational.api.Options;
2524
import com.apple.foundationdb.relational.utils.ResultSetAssert;
2625
import com.apple.foundationdb.relational.utils.SimpleDatabaseRule;
27-
2826
import org.assertj.core.api.Assertions;
2927
import org.junit.jupiter.api.BeforeAll;
3028
import org.junit.jupiter.api.BeforeEach;
@@ -54,7 +52,6 @@ public class JoinWithLimitTest {
5452
@RegisterExtension
5553
@Order(2)
5654
public final RelationalConnectionRule connection = new RelationalConnectionRule(db::getConnectionUri)
57-
.withOptions(Options.builder().withOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, true).build())
5855
.withSchema(db.getSchemaName());
5956

6057
@RegisterExtension

‎fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/ExplainTests.java‎

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,14 @@
2121
package com.apple.foundationdb.relational.recordlayer.query;
2222

2323
import com.apple.foundationdb.relational.api.Continuation;
24-
import com.apple.foundationdb.relational.api.Options;
25-
import com.apple.foundationdb.relational.api.RelationalConnection;
2624
import com.apple.foundationdb.relational.api.RelationalPreparedStatement;
2725
import com.apple.foundationdb.relational.api.RelationalResultSet;
2826
import com.apple.foundationdb.relational.api.RelationalStatement;
2927
import com.apple.foundationdb.relational.recordlayer.EmbeddedRelationalExtension;
3028
import com.apple.foundationdb.relational.recordlayer.Utils;
3129
import com.apple.foundationdb.relational.utils.Ddl;
32-
import com.apple.foundationdb.relational.utils.ResultSetAssert;
3330
import com.apple.foundationdb.relational.utils.RelationalStructAssert;
34-
31+
import com.apple.foundationdb.relational.utils.ResultSetAssert;
3532
import org.assertj.core.api.Assertions;
3633
import org.junit.jupiter.api.Order;
3734
import org.junit.jupiter.api.Test;
@@ -117,47 +114,12 @@ void explainWithNoContinuationTest() throws Exception {
117114
}
118115
}
119116

120-
@Test
121-
void explainWithContinuationNoSerializedPlanTest() throws Exception {
122-
try (var ddl = Ddl.builder().database(URI.create("/TEST/QT")).relationalExtension(relationalExtension).schemaTemplate(schemaTemplate).build()) {
123-
executeInsert(ddl);
124-
Continuation continuation;
125-
try (RelationalConnection conn = ddl.setSchemaAndGetConnection()) {
126-
conn.setOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, false);
127-
try (RelationalPreparedStatement ps = conn.prepareStatement("SELECT * FROM RestaurantComplexRecord")) {
128-
ps.setMaxRows(2);
129-
continuation = consumeResultAndGetContinuation(ps, 2);
130-
}
131-
try (RelationalPreparedStatement ps = conn.prepareStatement("EXPLAIN SELECT * FROM RestaurantComplexRecord WITH CONTINUATION ?cont")) {
132-
ps.setMaxRows(2);
133-
ps.setObject("cont", continuation.serialize());
134-
try (final RelationalResultSet resultSet = ps.executeQuery()) {
135-
final var assertResult = ResultSetAssert.assertThat(resultSet);
136-
137-
assertResult.hasNextRow()
138-
.hasColumn("PLAN", "ISCAN(RECORD_NAME_IDX <,>)")
139-
.hasColumn("PLAN_HASH", -1635569052);
140-
final var continuationInfo = resultSet.getStruct(5);
141-
org.junit.jupiter.api.Assertions.assertNotNull(continuationInfo);
142-
final var assertStruct = RelationalStructAssert.assertThat(continuationInfo);
143-
assertStruct.hasValue("EXECUTION_STATE", new byte[]{0, 21, 1, 21, 11});
144-
assertStruct.hasValue("VERSION", 1);
145-
assertStruct.hasValue("PLAN_HASH_MODE", null);
146-
assertStruct.hasValue("PLAN_HASH", -1635569052);
147-
assertStruct.hasValue("SERIALIZED_PLAN_COMPLEXITY", null);
148-
}
149-
}
150-
}
151-
}
152-
}
153-
154117
@Test
155118
void explainWithContinuationSerializedPlanTest() throws Exception {
156119
try (var ddl = Ddl.builder().database(URI.create("/TEST/QT")).relationalExtension(relationalExtension).schemaTemplate(schemaTemplate).build()) {
157120
executeInsert(ddl);
158121
Continuation continuation;
159122
try (final var connection = ddl.setSchemaAndGetConnection()) {
160-
connection.setOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, true);
161123
try (RelationalPreparedStatement ps = ddl.setSchemaAndGetConnection().prepareStatement("SELECT * FROM RestaurantComplexRecord")) {
162124
ps.setMaxRows(2);
163125
continuation = consumeResultAndGetContinuation(ps, 2);
@@ -190,7 +152,6 @@ void explainWithContinuationSerializedPlanWithDifferentQueryTest() throws Except
190152
executeInsert(ddl);
191153
Continuation continuation;
192154
try (final var connection = ddl.setSchemaAndGetConnection()) {
193-
connection.setOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, true);
194155
try (RelationalPreparedStatement ps = ddl.setSchemaAndGetConnection().prepareStatement("SELECT * FROM RestaurantComplexRecord")) {
195156
ps.setMaxRows(2);
196157
continuation = consumeResultAndGetContinuation(ps, 2);
@@ -223,7 +184,6 @@ void explainExecuteStatementTest() throws Exception {
223184
executeInsert(ddl);
224185
Continuation continuation;
225186
try (final var connection = ddl.setSchemaAndGetConnection()) {
226-
connection.setOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, true);
227187
try (RelationalPreparedStatement ps = ddl.setSchemaAndGetConnection().prepareStatement("SELECT * FROM RestaurantComplexRecord")) {
228188
ps.setMaxRows(2);
229189
continuation = consumeResultAndGetContinuation(ps, 2);

‎fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/ForceContinuationQueryTests.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,13 @@
2121
package com.apple.foundationdb.relational.recordlayer.query;
2222

2323
import com.apple.foundationdb.relational.api.Continuation;
24-
import com.apple.foundationdb.relational.api.Options;
2524
import com.apple.foundationdb.relational.api.exceptions.ContextualSQLException;
2625
import com.apple.foundationdb.relational.recordlayer.EmbeddedRelationalExtension;
2726
import com.apple.foundationdb.relational.recordlayer.RelationalConnectionRule;
2827
import com.apple.foundationdb.relational.recordlayer.RelationalStatementRule;
2928
import com.apple.foundationdb.relational.recordlayer.UniqueIndexTests;
3029
import com.apple.foundationdb.relational.recordlayer.Utils;
3130
import com.apple.foundationdb.relational.utils.SimpleDatabaseRule;
32-
3331
import org.junit.jupiter.api.Assertions;
3432
import org.junit.jupiter.api.BeforeAll;
3533
import org.junit.jupiter.api.BeforeEach;
@@ -40,7 +38,6 @@
4038
import org.junit.jupiter.params.provider.Arguments;
4139
import org.junit.jupiter.params.provider.MethodSource;
4240

43-
import java.sql.SQLException;
4441
import java.util.stream.Stream;
4542

4643
public class ForceContinuationQueryTests {
@@ -75,14 +72,13 @@ public class ForceContinuationQueryTests {
7572
@RegisterExtension
7673
@Order(2)
7774
public final RelationalConnectionRule connection = new RelationalConnectionRule(db::getConnectionUri)
78-
.withOptions(Options.builder().withOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, true).build())
7975
.withSchema(db.getSchemaName());
8076

8177
@RegisterExtension
8278
@Order(3)
8379
public final RelationalStatementRule statement = new RelationalStatementRule(connection);
8480

85-
public ForceContinuationQueryTests() throws SQLException {
81+
public ForceContinuationQueryTests() {
8682
}
8783

8884
@BeforeAll

‎fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/GroupByQueryTests.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ void groupByWithScanLimit() throws Exception {
5959
try (var conn = ddl.setSchemaAndGetConnection()) {
6060
Continuation continuation = null;
6161
conn.setOption(Options.Name.EXECUTION_SCANNED_ROWS_LIMIT, 2);
62-
conn.setOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, true);
6362
try (var statement = conn.createStatement()) {
6463
insertT1Record(statement, 2, 1, 1, 20);
6564
insertT1Record(statement, 3, 1, 2, 5);
@@ -82,7 +81,6 @@ void groupByWithScanLimit() throws Exception {
8281
}
8382
try (var preparedStatement = conn.prepareStatement("EXECUTE CONTINUATION ?param")) {
8483
conn.setOption(Options.Name.EXECUTION_SCANNED_ROWS_LIMIT, 2);
85-
conn.setOption(Options.Name.CONTINUATIONS_CONTAIN_COMPILED_STATEMENTS, true);
8684
// scan pk = 5 and pk = 4 rows, hit SCAN_LIMIT_REACHED
8785
preparedStatement.setBytes("param", continuation.serialize());
8886
try (final RelationalResultSet resultSet = preparedStatement.executeQuery()) {

0 commit comments

Comments
 (0)