Skip to content

Commit 49bfe50

Browse files
alecgrieserScottDugas
alecgrieser
and
ScottDugas
authored
Adds commands to the YAML framework to assert on changing query behavior (#3188)
This adds additional arguments to the YAML test framework to assert on changing query behavior. For example, if a query used to return fewer columns and that information is baked into a plan, we might expect the configuration to look something like: ```yaml - - query: SELECT something() - initialVersionLessThan: 4.2.0.0 - result: [{a: 1, b: 2}, {a: 3, b: 4}] - initialVersionAtLeast: 4.2.0.0 - result: [{a: 1, b: 2, c: "foo"}, {a: 3, b: 4, c: "bar"}] ``` The way to read this is that after the results following `initialVersionLessThan: 4.2.0.0` should be returned as long as the query starts on a version less than `4.2.0.0`. If we having continuations (like if we are running in `force_continuation` mode), then we get coverage of what happens when we send the continuation up to a new version, which may be 4.2.0.0+. Likewise, the latter result (returning more columns) is expected even if the continuation is sent back to a pre-4.2.0.0 version. In this PR, I've also updated the YAML tests that were resulting in mixed-mode failures between 4.1.6.0 and earlier versions so that it now takes the version into account. This shows the explicit behavior change that we are now asserting on that previously would have resulted in queries that were only run on the new version. This resolves #3114. --------- Co-authored-by: Scott Dugas <[email protected]>
1 parent c4f19e5 commit 49bfe50

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+1481
-185
lines changed

‎build.gradle

+1
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ subprojects {
279279
sourceSets["test"].resources.srcDirs.stream().flatMap( resourceDir ->
280280
fileTree(dir: resourceDir).matching {
281281
include '**/*.yamsql'
282+
exclude '/initial-version/*'
282283
exclude '/supported-version/*'
283284
}.stream()
284285
).forEach { yamsql ->

‎scripts/YAML-SQL.xml

+3-2
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@
5151
<keywords3 keywords="false;null;true;ordered;randomized;parallelized;test;block;single_repetition_ordered;
5252
single_repetition_randomized;single_repetition_parallelized;multi_repetition_ordered;multi_repetition_randomized;
5353
multi_repetition_parallelized" />
54-
<keywords4 keywords="connect:;query:;load schema template:;set schema state:;result:;unorderedresult:;explain:;
55-
explaincontains:;count:;error:;planhash:;setup:;schema_template:;supported_version:;test_block:;options:;tests:;mode:;repetition:;seed:;
54+
<keywords4 keywords="connect:;query:;load schema template:;set schema state:;result:;unorderedResult:;explain:;
55+
explainContains:;count:;error:;planHash:;setup:;schema_template:;supported_version:;test_block:;options:;tests:;
56+
maxRows:;initialVersionAtLeast:;initialVersionLessThan:;;mode:;repetition:;seed:;
5657
check_cache:;connection_lifecycle:;steps:;preset:;statement_type:;!r;!in;!a;" />
5758
</highlighting>
5859
<extensionMap>

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/CustomYamlConstructor.java

+2
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ public CustomYamlConstructor(LoaderOptions loaderOptions) {
7575
requireLineNumber.add(QueryConfig.QUERY_CONFIG_PLAN_HASH);
7676
requireLineNumber.add(QueryConfig.QUERY_CONFIG_MAX_ROWS);
7777
requireLineNumber.add(QueryConfig.QUERY_CONFIG_SUPPORTED_VERSION);
78+
requireLineNumber.add(QueryConfig.QUERY_CONFIG_INITIAL_VERSION_LESS_THAN);
79+
requireLineNumber.add(QueryConfig.QUERY_CONFIG_INITIAL_VERSION_AT_LEAST);
7880
}
7981

8082
@Override

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/MultiServerConnectionFactory.java

+18-11
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.apple.foundationdb.relational.api.metrics.MetricCollector;
2626
import com.apple.foundationdb.relational.recordlayer.EmbeddedRelationalConnection;
2727
import com.apple.foundationdb.relational.util.Assert;
28+
import com.apple.foundationdb.relational.yamltests.server.SemanticVersion;
2829
import org.apache.logging.log4j.LogManager;
2930
import org.apache.logging.log4j.Logger;
3031
import org.junit.jupiter.api.Assertions;
@@ -49,7 +50,7 @@
4950
public class MultiServerConnectionFactory implements YamlConnectionFactory {
5051
// The fixed index of the default connection
5152
public static final int DEFAULT_CONNECTION = 0;
52-
private final Set<String> versionsUnderTest;
53+
private final Set<SemanticVersion> versionsUnderTest;
5354

5455
/**
5556
* Server selection policy.
@@ -96,7 +97,7 @@ public YamlConnection getNewConnection(@Nonnull URI connectPath) throws SQLExcep
9697
}
9798

9899
@Override
99-
public Set<String> getVersionsUnderTest() {
100+
public Set<SemanticVersion> getVersionsUnderTest() {
100101
return versionsUnderTest;
101102
}
102103

@@ -148,7 +149,7 @@ public static class MultiServerConnection implements YamlConnection {
148149
@Nonnull
149150
private final List<YamlConnection> underlyingConnections;
150151
@Nonnull
151-
private final List<String> versions;
152+
private final List<SemanticVersion> versions;
152153

153154
public MultiServerConnection(@Nonnull ConnectionSelectionPolicy connectionSelectionPolicy,
154155
final int initialConnecion,
@@ -188,10 +189,16 @@ public EmbeddedRelationalConnection tryGetEmbedded() {
188189

189190
@Nonnull
190191
@Override
191-
public List<String> getVersions() {
192+
public List<SemanticVersion> getVersions() {
192193
return this.versions;
193194
}
194195

196+
@Nonnull
197+
@Override
198+
public SemanticVersion getInitialVersion() {
199+
return versions.get(0);
200+
}
201+
195202
@Override
196203
public void close() throws SQLException {
197204
logger.info("Sending operation {} to all connections", "close");
@@ -228,16 +235,16 @@ private YamlConnection getCurrentConnection(boolean advance, String op) {
228235
throw new IllegalStateException("Unsupported selection policy " + connectionSelectionPolicy);
229236
}
230237

231-
private static List<String> createVersionsList(final int initialConnecion,
232-
final List<YamlConnection> relationalConnections) {
233-
List<String> versions = new ArrayList<>();
234-
for (int i = initialConnecion; i < relationalConnections.size(); i++) {
235-
final List<String> underlying = relationalConnections.get(i).getVersions();
238+
private static List<SemanticVersion> createVersionsList(final int initialConnection,
239+
final List<YamlConnection> relationalConnections) {
240+
List<SemanticVersion> versions = new ArrayList<>();
241+
for (int i = initialConnection; i < relationalConnections.size(); i++) {
242+
final List<SemanticVersion> underlying = relationalConnections.get(i).getVersions();
236243
Assert.thatUnchecked(underlying.size() == 1, "Part of multi server config has more than one version");
237244
versions.add(underlying.get(0));
238245
}
239-
for (int i = 0; i < initialConnecion; i++) {
240-
final List<String> underlying = relationalConnections.get(i).getVersions();
246+
for (int i = 0; i < initialConnection; i++) {
247+
final List<SemanticVersion> underlying = relationalConnections.get(i).getVersions();
241248
Assert.thatUnchecked(underlying.size() == 1, "Part of multi server config has more than one version");
242249
versions.add(underlying.get(0));
243250
}

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/SimpleYamlConnection.java

+10-3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.apple.foundationdb.relational.api.RelationalStatement;
2626
import com.apple.foundationdb.relational.api.metrics.MetricCollector;
2727
import com.apple.foundationdb.relational.recordlayer.EmbeddedRelationalConnection;
28+
import com.apple.foundationdb.relational.yamltests.server.SemanticVersion;
2829

2930
import javax.annotation.Nonnull;
3031
import javax.annotation.Nullable;
@@ -39,9 +40,9 @@ public class SimpleYamlConnection implements YamlConnection {
3940
@Nonnull
4041
private final RelationalConnection underlying;
4142
@Nonnull
42-
private final List<String> versions;
43+
private final List<SemanticVersion> versions;
4344

44-
public SimpleYamlConnection(@Nonnull Connection connection, @Nonnull String version) throws SQLException {
45+
public SimpleYamlConnection(@Nonnull Connection connection, @Nonnull SemanticVersion version) throws SQLException {
4546
underlying = connection.unwrap(RelationalConnection.class);
4647
this.versions = List.of(version);
4748
}
@@ -84,7 +85,13 @@ public EmbeddedRelationalConnection tryGetEmbedded() {
8485

8586
@Nonnull
8687
@Override
87-
public List<String> getVersions() {
88+
public List<SemanticVersion> getVersions() {
8889
return versions;
8990
}
91+
92+
@Nonnull
93+
@Override
94+
public SemanticVersion getInitialVersion() {
95+
return versions.get(0);
96+
}
9097
}

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlConnection.java

+15-6
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.apple.foundationdb.relational.api.RelationalStatement;
2626
import com.apple.foundationdb.relational.api.metrics.MetricCollector;
2727
import com.apple.foundationdb.relational.recordlayer.EmbeddedRelationalConnection;
28+
import com.apple.foundationdb.relational.yamltests.server.SemanticVersion;
2829

2930
import javax.annotation.Nonnull;
3031
import javax.annotation.Nullable;
@@ -35,11 +36,6 @@
3536
* A wrapper around {@link java.sql.Connection} to support yaml tests.
3637
*/
3738
public interface YamlConnection extends AutoCloseable {
38-
/**
39-
* String for representing the current version of the code.
40-
*/
41-
String CURRENT_VERSION = "!currentVersion";
42-
4339
/**
4440
* Close this connection.
4541
* @throws SQLException if there was an issue closing the underlying connection(s).
@@ -91,5 +87,18 @@ public interface YamlConnection extends AutoCloseable {
9187
* @return the ordered list of versions
9288
*/
9389
@Nonnull
94-
List<String> getVersions();
90+
List<SemanticVersion> getVersions();
91+
92+
/**
93+
* Return the initial version returned by this connection. If this connection
94+
* wraps multiple versions, it may return different underlying connections
95+
* with every call to {@link #createStatement()}. This returns the version
96+
* associated with the first such call, which can impact the set of results
97+
* that we expect to return as that is also the connection that should be
98+
* used for query planning.
99+
*
100+
* @return the first version that an underlying connection will represent
101+
*/
102+
@Nonnull
103+
SemanticVersion getInitialVersion();
95104
}

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlConnectionFactory.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
package com.apple.foundationdb.relational.yamltests;
2222

2323
import com.apple.foundationdb.relational.api.RelationalConnection;
24+
import com.apple.foundationdb.relational.yamltests.server.SemanticVersion;
2425

2526
import javax.annotation.Nonnull;
2627
import java.net.URI;
@@ -53,7 +54,7 @@ public interface YamlConnectionFactory {
5354
* @return A set of versions that we are testing against, or an empty set if just testing against the current
5455
* version
5556
*/
56-
Set<String> getVersionsUnderTest();
57+
Set<SemanticVersion> getVersionsUnderTest();
5758

5859
/**
5960
* Whether the connection supports multiple servers.

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/block/FileOptions.java

+14-3
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.apple.foundationdb.relational.yamltests.CustomYamlConstructor;
2424
import com.apple.foundationdb.relational.yamltests.Matchers;
2525
import com.apple.foundationdb.relational.yamltests.YamlExecutionContext;
26+
import com.apple.foundationdb.relational.yamltests.server.SemanticVersion;
2627
import com.apple.foundationdb.relational.yamltests.server.SupportedVersionCheck;
2728
import org.apache.logging.log4j.LogManager;
2829
import org.apache.logging.log4j.Logger;
@@ -55,8 +56,7 @@ public class FileOptions {
5556

5657
public static Block parse(int lineNumber, Object document, YamlExecutionContext executionContext) {
5758
final Map<?, ?> options = CustomYamlConstructor.LinedObject.unlineKeys(Matchers.map(document, OPTIONS));
58-
Object rawVersion = options.get(SUPPORTED_VERSION_OPTION);
59-
final SupportedVersionCheck check = SupportedVersionCheck.parse(rawVersion, executionContext);
59+
final SupportedVersionCheck check = SupportedVersionCheck.parseOptions(options, executionContext);
6060
if (!check.isSupported()) {
6161
// IntelliJ, at least, doesn't display the reason, so log it
6262
if (logger.isInfoEnabled()) {
@@ -67,15 +67,26 @@ public static Block parse(int lineNumber, Object document, YamlExecutionContext
6767
return new NoOpBlock(lineNumber);
6868
}
6969

70+
public static SemanticVersion parseVersion(Object rawVersion) {
71+
if (rawVersion instanceof CurrentVersion) {
72+
return SemanticVersion.current();
73+
} else if (rawVersion instanceof String) {
74+
return SemanticVersion.parse((String) rawVersion);
75+
} else {
76+
throw new IllegalArgumentException("Unable to determine semantic version from object: " + rawVersion);
77+
}
78+
}
79+
7080
public static final class CurrentVersion {
7181
public static final CurrentVersion INSTANCE = new CurrentVersion();
82+
public static final String TEXT = SemanticVersion.SemanticVersionType.CURRENT.getText();
7283

7384
private CurrentVersion() {
7485
}
7586

7687
@Override
7788
public String toString() {
78-
return "!current_version";
89+
return TEXT;
7990
}
8091
}
8192
}

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/block/TestBlock.java

+6-12
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ public final class TestBlock extends ConnectedBlock {
104104
static final String OPTION_STATEMENT_TYPE = "statement_type";
105105
static final String OPTION_STATEMENT_TYPE_SIMPLE = "simple";
106106
static final String OPTION_STATEMENT_TYPE_PREPARED = "prepared";
107-
static final String OPTION_SUPPORTED_VERSION = FileOptions.SUPPORTED_VERSION_OPTION;
108107

109108
/**
110109
* Defines the way in which the tests are run.
@@ -190,7 +189,7 @@ private static class TestBlockOptions {
190189
private boolean checkCache = true;
191190
private ConnectionLifecycle connectionLifecycle = ConnectionLifecycle.TEST;
192191
private StatementType statementType = StatementType.BOTH;
193-
private Object rawSupportedVersion;
192+
private SupportedVersionCheck supportedVersionCheck = SupportedVersionCheck.supported();
194193

195194
private void verifyPreset(@Nonnull String preset) {
196195
switch (preset) {
@@ -224,7 +223,7 @@ private void setWithPreset(@Nonnull String preset) {
224223
}
225224
}
226225

227-
private void setWithOptionsMap(@Nonnull Map<?, ?> optionsMap) {
226+
private void setWithOptionsMap(@Nonnull Map<?, ?> optionsMap, @Nonnull YamlExecutionContext executionContext) {
228227
setOptionExecutionModeAndRepetition(optionsMap);
229228
if (optionsMap.containsKey(OPTION_SEED)) {
230229
this.seed = Matchers.longValue(optionsMap.get(OPTION_SEED));
@@ -246,9 +245,7 @@ private void setWithOptionsMap(@Nonnull Map<?, ?> optionsMap) {
246245
break;
247246
}
248247
}
249-
if (optionsMap.containsKey(OPTION_SUPPORTED_VERSION)) {
250-
this.rawSupportedVersion = optionsMap.get(OPTION_SUPPORTED_VERSION);
251-
}
248+
supportedVersionCheck = SupportedVersionCheck.parseOptions(optionsMap, executionContext);
252249
setOptionConnectionLifecycle(optionsMap);
253250
}
254251

@@ -330,19 +327,16 @@ public static Block parse(int blockNumber, int lineNumber, @Nonnull Object docum
330327
}
331328
// higher priority than the preset is that of options map, set the options according to that, if any is present.
332329
if (testsMap.get(TEST_BLOCK_OPTIONS) != null) {
333-
options.setWithOptionsMap(CustomYamlConstructor.LinedObject.unlineKeys(Matchers.map(testsMap.get(TEST_BLOCK_OPTIONS))));
330+
options.setWithOptionsMap(CustomYamlConstructor.LinedObject.unlineKeys(Matchers.map(testsMap.get(TEST_BLOCK_OPTIONS))), executionContext);
334331
}
335332
// execution context carries the highest priority, try setting options per that if it has some options to override.
336333
options.setWithExecutionContext(executionContext);
337334

338335
final String blockName = testsMap.containsKey(TEST_BLOCK_NAME)
339336
? Matchers.string(testsMap.get(TEST_BLOCK_NAME)) : "unnamed-" + blockNumber;
340337
final var testsObject = Matchers.notNull(testsMap.get(TEST_BLOCK_TESTS), "‼️ tests not found at line " + lineNumber);
341-
if (options.rawSupportedVersion != null) {
342-
final SupportedVersionCheck check = SupportedVersionCheck.parse(options.rawSupportedVersion, executionContext);
343-
if (!check.isSupported()) {
344-
return new SkipBlock(lineNumber, check.getMessage());
345-
}
338+
if (!options.supportedVersionCheck.isSupported()) {
339+
return new SkipBlock(lineNumber, options.supportedVersionCheck.getMessage());
346340
}
347341
var randomGenerator = new Random(options.seed);
348342
final var executables = new ArrayList<Consumer<YamlConnection>>();

‎yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/QueryCommand.java

+16-8
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,10 @@ public static Command parse(@Nonnull final Object object, @Nonnull final String
8787
}
8888
final var queryString = Matchers.notNull(Matchers.string(queryCommand.getValue(), "query string"), "query string");
8989
final var queryInterpreter = QueryInterpreter.withQueryString(lineNumber, queryString, executionContext);
90-
final var queryConfigsWithValueList = Matchers.arrayList(object).stream().skip(1).collect(Collectors.toList());
90+
final List<?> queryConfigsWithValueList = Matchers.arrayList(object).stream().skip(1).collect(Collectors.toList());
9191
final var configs = queryConfigsWithValueList.isEmpty() ?
9292
List.of(QueryConfig.getNoCheckConfig(lineNumber, executionContext)) :
93-
queryConfigsWithValueList.stream().map(l -> QueryConfig.parse(l, blockName, executionContext)).collect(Collectors.toList());
94-
95-
Assert.thatUnchecked(configs.stream().skip(1)
96-
.noneMatch(config -> QueryConfig.QUERY_CONFIG_SUPPORTED_VERSION.equals(config.getConfigName())),
97-
"supported_version must be the first config in a query (after the query itself)");
93+
QueryConfig.parseConfigs(blockName, lineNumber, queryConfigsWithValueList, executionContext);
9894

9995
final List<QueryConfig> skipConfigs = configs.stream().filter(config -> config instanceof QueryConfig.SkipConfig)
10096
.collect(Collectors.toList());
@@ -156,13 +152,25 @@ void executeInternal(@Nonnull final YamlConnection connection) throws SQLExcepti
156152
private void executeInternal(@Nonnull final YamlConnection connection, boolean checkCache, @Nonnull QueryExecutor executor)
157153
throws SQLException, RelationalException {
158154
enableCascadesDebugger();
155+
boolean shouldExecute = true;
159156
boolean queryIsRunning = false;
160157
Continuation continuation = null;
161158
Integer maxRows = null;
162159
boolean exhausted = false;
160+
boolean errored = false;
163161
var queryConfigsIterator = queryConfigs.listIterator();
164162
while (queryConfigsIterator.hasNext()) {
165163
var queryConfig = queryConfigsIterator.next();
164+
if (queryConfig instanceof QueryConfig.InitialVersionCheckConfig) {
165+
Assert.thatUnchecked(!queryIsRunning, "Initial version checks should not be executed while a query is running");
166+
shouldExecute = ((QueryConfig.InitialVersionCheckConfig)queryConfig).shouldExecute(connection);
167+
continue;
168+
}
169+
if (!shouldExecute) {
170+
continue;
171+
}
172+
173+
Assert.that(!errored, "ERROR config should be the last config specified.");
166174
if (QueryConfig.QUERY_CONFIG_MAX_ROWS.equals(queryConfig.getConfigName())) {
167175
maxRows = Integer.parseInt(queryConfig.getValueString());
168176
} else if (QueryConfig.QUERY_CONFIG_PLAN_HASH.equals(queryConfig.getConfigName())) {
@@ -173,7 +181,7 @@ private void executeInternal(@Nonnull final YamlConnection connection, boolean c
173181
Integer finalMaxRows = maxRows;
174182
runWithDebugger(() -> executor.execute(connection, null, queryConfig, checkCache, finalMaxRows));
175183
} else if (QueryConfig.QUERY_CONFIG_EXPLAIN.equals(queryConfig.getConfigName()) || QueryConfig.QUERY_CONFIG_EXPLAIN_CONTAINS.equals(queryConfig.getConfigName())) {
176-
Assert.thatUnchecked(!queryIsRunning, "Explain test should not be intermingled with query result tests");
184+
Assert.that(!queryIsRunning, "Explain test should not be intermingled with query result tests");
177185
// ignore debugger configuration, always set the debugger for explain, so we can always get consistent
178186
// results
179187
Integer finalMaxRows1 = maxRows;
@@ -183,7 +191,7 @@ private void executeInternal(@Nonnull final YamlConnection connection, boolean c
183191
continue;
184192
} else if (!QueryConfig.QUERY_CONFIG_SUPPORTED_VERSION.equals(queryConfig.getConfigName())) {
185193
if (QueryConfig.QUERY_CONFIG_ERROR.equals(queryConfig.getConfigName())) {
186-
Assert.that(!queryConfigsIterator.hasNext(), "ERROR config should be the last config specified.");
194+
errored = true;
187195
}
188196

189197
if (exhausted && (QueryConfig.QUERY_CONFIG_RESULT.equals(queryConfig.getConfigName()) || QueryConfig.QUERY_CONFIG_UNORDERED_RESULT.equals(queryConfig.getConfigName()))) {

0 commit comments

Comments
 (0)