Skip to content

Commit 11494c3

Browse files
alecgrieseralecgrieser
committed
Squashed commit of the following:
commit 9a8192c Author: Alec Grieser <agrieser@apple.com> Date: Thu Feb 27 11:27:53 2025 +0000 update bitmap-aggregate-index.yamsql using new feature and enable force_continuations commit 169e582 Author: Alec Grieser <agrieser@apple.com> Date: Thu Feb 27 10:09:05 2025 +0000 fix up failing MultiServerConnectionFactoryTest to use SemanticVersions instead of Strings commit fe4602f Merge: 7d61714 8a3c17f Author: Alec Grieser <agrieser@apple.com> Date: Wed Feb 26 18:56:11 2025 +0000 Merge remote-tracking branch 'upstream/main' into 03114-fine-grained-multi-version commit 7d61714 Author: Alec Grieser <agrieser@apple.com> Date: Wed Feb 26 18:52:10 2025 +0000 combine CodeVersion implementations into SemanticVersion commit 8a3c17f Author: Alec Grieser <agrieser@apple.com> Date: Wed Feb 26 18:31:21 2025 +0000 Run the PR label check whenever the HEAD commit changes to ensure it is applied to the final commit of each PR (FoundationDB#3195) This makes sure the PR label check is run every time the commit changes on a PR. Strictly speaking, this seems like it shouldn't be necessary, but if it isn't added, then the check may not be applied to the final commit in a PR, which in turn means that the check won't be there to block a PR missing an appropriate label. commit 993a9b4 Author: Alec Grieser <agrieser@apple.com> Date: Wed Feb 26 17:53:40 2025 +0000 Address comments from @sdugas: * Use CodeVersion class in more places * Add tests for !current_version in the initial-version yaml tests * Remove vestigial weirdness from previous iterations of this code * Update YAML-SQL.xml with the new syntax commit 25a0c2a Author: Alec Grieser <agrieser@apple.com> Date: Wed Feb 26 11:47:25 2025 +0000 update so that we parse CodeVersion sooner in more places commit 7a9f978 Merge: 5c5a9c5 d736d89 Author: Alec Grieser <agrieser@apple.com> Date: Tue Feb 25 19:05:44 2025 +0000 Merge remote-tracking branch 'upstream/main' into 03114-fine-grained-multi-version commit 5c5a9c5 Author: Alec Grieser <agrieser@apple.com> Date: Tue Feb 25 19:05:23 2025 +0000 move the concept of \"current_version\" out of `SemanticVersion` commit 376f08b Author: Alec Grieser <agrieser@apple.com> Date: Mon Feb 24 19:49:32 2025 +0000 fix up failing tests commit 360e006 Author: Alec Grieser <agrieser@apple.com> Date: Mon Feb 24 19:37:43 2025 +0000 mark SUPPORTED as final commit 131742d Author: Alec Grieser <agrieser@apple.com> Date: Mon Feb 24 19:29:22 2025 +0000 handle some merge skew commit 48bb24e Author: Alec Grieser <agrieser@apple.com> Date: Mon Feb 24 19:14:44 2025 +0000 remove weird contains in favor of equals commit 2345d3f Author: Alec Grieser <agrieser@apple.com> Date: Mon Feb 24 19:14:36 2025 +0000 make recursive cte supported_versions more precise commit 918afd9 Author: Alec Grieser <agrieser@apple.com> Date: Mon Feb 24 18:55:42 2025 +0000 * Add test casese for the initial version configurations to validate tests get run * Make sure that the versions specified are comprehensive commit 4f792a6 Author: Alec Grieser <agrieser@apple.com> Date: Mon Feb 24 14:41:50 2025 +0000 restore requirement that supported_version is first non-query command commit ae86f93 Author: Alec Grieser <agrieser@apple.com> Date: Mon Feb 24 14:19:48 2025 +0000 base version config checking off of runtime determination commit 263d71f Author: Alec Grieser <agrieser@apple.com> Date: Mon Feb 24 11:24:21 2025 +0000 back out changes to have connection factory support initial version commit c3ff258 Author: Alec Grieser <agrieser@apple.com> Date: Fri Feb 21 17:49:21 2025 +0000 update recursive cte supported version commit 228d6b5 Author: Alec Grieser <agrieser@apple.com> Date: Fri Feb 21 16:37:30 2025 +0000 revert yaml-tests.gradle to main commit fec41c9 Author: Alec Grieser <agrieser@apple.com> Date: Fri Feb 21 16:04:23 2025 +0000 fix up mixed mode tests in way that shows new configs commit 529ae67 Author: Alec Grieser <agrieser@apple.com> Date: Fri Feb 21 15:46:46 2025 +0000 update MultiServerConnectionFactory so it is reset for every query commit 869f6d4 Author: Alec Grieser <agrieser@apple.com> Date: Fri Feb 21 14:53:43 2025 +0000 add framework for choosing results by version commit 3a1fb3f Author: Alec Grieser <agrieser@apple.com> Date: Thu Feb 20 18:15:33 2025 +0000 abandon plans to have multiple supported version options commit 6e8d089 Author: Alec Grieser <agrieser@apple.com> Date: Fri Feb 7 15:53:12 2025 +0000 * Add YamlExecutionContext state about the initial version chosen for each query * Modify the SupportedVersionCheck parsing so that it can accept multiple fields and doesn't need to be the first config
1 parent 06f4606 commit 11494c3

Some content is hidden

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

46 files changed

+1516
-202
lines changed

‎.github/workflows/pr_labels.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ name: Pull Request Labels
22

33
on:
44
pull_request:
5-
types: [opened, labeled, unlabeled]
5+
# Run whenever the labels change or the PR HEAD changes. In theory, the latter is a bit redundant,
6+
# but it's necessary to make sure that the check is applied to the final commit for a PR
7+
types: [opened, reopened, labeled, unlabeled, synchronize]
68

79
jobs:
810
labels:

‎build.gradle

Lines changed: 1 addition & 0 deletions
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

Lines changed: 3 additions & 2 deletions
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

Lines changed: 2 additions & 0 deletions
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

Lines changed: 18 additions & 11 deletions
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

Lines changed: 10 additions & 3 deletions
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

Lines changed: 15 additions & 6 deletions
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

Lines changed: 2 additions & 1 deletion
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

Lines changed: 14 additions & 3 deletions
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

Lines changed: 6 additions & 12 deletions
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>>();

0 commit comments

Comments
 (0)