Merge pull request #3031 from ScottDugas/mixed-handle-new-features

Ability to prevent tests from running against old versions.

With the new mixed-mode tests being run as part of PRB, we'll need a way to say that a test should not run against old versions. This adds support for disabling an entire yaml file. Future steps would be to allow disabling a specific query, providing a specific supported version, and also having the build update the `!current_version` to the version being released.

Author: ScottDugas

scripts/YAML-SQL.xml

@@ -47,15 +47,15 @@
     timezone_;timezone_minute;to;trailing;transaction;translate;translation;trim;type;union;unique;unknown;update;
     upper;usage;user;using;value;values;varying;view;when;whenever;where;with;work;write;year;zone" ignore_case="true" />
     <keywords2 keywords="bigint;bit;boolean;bytes;char;char_;character;date;dec;decimal;double;float;int;integer;nchar;
-    numeric;real;smallint;string;time;timestamp;varchar;!!;!r;!in;!a;" />
+    numeric;real;smallint;string;time;timestamp;varchar;!!;!r;!in;!a;!current_version" />
     <keywords3 keywords="false;null;true;ordered;randomized;parallelized;test;block;single_repetition_ordered;
     single_repetition_randomized;single_repetition_parallelized;multi_repetition_ordered;multi_repetition_randomized;
     multi_repetition_parallelized" />
     <keywords4 keywords="connect:;query:;load schema template:;set schema state:;result:;unorderedresult:;explain:;
-    explaincontains:;count:;error:;planhash:;setup:;schema_template:;test_block:;options:;tests:;mode:;repetition:;seed:;
+    explaincontains:;count:;error:;planhash:;setup:;schema_template:;supported_version:;test_block:;options:;tests:;mode:;repetition:;seed:;
     check_cache:;connection_lifecycle:;steps:;preset:;statement_type:;!r;!in;!a;" />
   </highlighting>
   <extensionMap>
     <mapping ext="yamsql" />
   </extensionMap>
-</filetype>
+</filetype>

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

@@ -22,6 +22,7 @@
 
 import com.apple.foundationdb.relational.api.exceptions.ErrorCode;
 import com.apple.foundationdb.relational.util.Assert;
+import com.apple.foundationdb.relational.yamltests.block.FileOptions;
 import com.apple.foundationdb.relational.yamltests.block.SetupBlock;
 import com.apple.foundationdb.relational.yamltests.block.TestBlock;
 import com.apple.foundationdb.relational.yamltests.command.Command;
@@ -34,10 +35,13 @@
 import org.yaml.snakeyaml.nodes.ScalarNode;
 import org.yaml.snakeyaml.nodes.Tag;
 
-import javax.annotation.Nonnull;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
+
+import javax.annotation.Nonnull;
 
 public class CustomYamlConstructor extends SafeConstructor {
 
@@ -51,8 +55,10 @@ public CustomYamlConstructor(LoaderOptions loaderOptions) {
         yamlConstructors.put(new Tag("!sc"), new ConstructStringContains());
         yamlConstructors.put(new Tag("!null"), new ConstructNullPlaceholder());
         yamlConstructors.put(new Tag("!not_null"), new ConstructNotNull());
+        yamlConstructors.put(new Tag("!current_version"), new ConstructCurrentVersion());
 
         //blocks
+        requireLineNumber.add(FileOptions.OPTIONS);
         requireLineNumber.add(SetupBlock.SETUP_BLOCK);
         requireLineNumber.add(SetupBlock.SchemaTemplateBlock.SCHEMA_TEMPLATE_BLOCK);
         requireLineNumber.add(TestBlock.TEST_BLOCK);
@@ -93,6 +99,24 @@ private LinedObject(final Object object, final int lineNumber) {
             this.lineNumber = lineNumber;
         }
 
+        /**
+         * Remove the {@link LinedObject} wrappers from keys and create a new map.
+         * @param blockMap a map from a yaml file that may have keys that had lines added
+         * @return a new map where the keys do not have lines added
+         */
+        public static Map<?, ?> unlineKeys(Map<?, ?> blockMap) {
+            return blockMap.entrySet().stream()
+                    .collect(Collectors.toMap(
+                            entry -> {
+                                if (entry.getKey() instanceof LinedObject) {
+                                    return ((LinedObject)entry.getKey()).getObject();
+                                } else {
+                                    return entry.getKey();
+                                }
+                            },
+                            Map.Entry::getValue));
+        }
+
         @Nonnull
         public Object getObject() {
             return object;
@@ -148,4 +172,12 @@ public Object construct(Node node) {
             return CustomTag.NotNull.INSTANCE;
         }
     }
+
+    private static class ConstructCurrentVersion extends AbstractConstruct {
+
+        @Override
+        public Object construct(Node node) {
+            return FileOptions.CurrentVersion.INSTANCE;
+        }
+    }
 }

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

@@ -28,15 +28,16 @@
 import org.apache.logging.log4j.Logger;
 
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 import java.net.URI;
 import java.sql.Array;
 import java.sql.DatabaseMetaData;
 import java.sql.SQLException;
 import java.sql.Struct;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * A connection factory that creates a connection that can be used with multiple servers.
@@ -47,6 +48,7 @@
 public class MultiServerConnectionFactory implements YamlRunner.YamlConnectionFactory {
     // The fixed index of the default connection
     public static final int DEFAULT_CONNECTION = 0;
+    private final Set<String> versionsUnderTest;
 
     /**
      * Server selection policy.
@@ -60,42 +62,47 @@ public enum ConnectionSelectionPolicy { DEFAULT, ALTERNATE }
     private final int initialConnection;
     @Nonnull
     private final YamlRunner.YamlConnectionFactory defaultFactory;
-    @Nullable
+    @Nonnull
     private final List<YamlRunner.YamlConnectionFactory> alternateFactories;
 
     public MultiServerConnectionFactory(@Nonnull final YamlRunner.YamlConnectionFactory defaultFactory,
-                                        @Nullable final List<YamlRunner.YamlConnectionFactory> alternateFactories) {
+                                        @Nonnull final List<YamlRunner.YamlConnectionFactory> alternateFactories) {
         this(ConnectionSelectionPolicy.DEFAULT, 0, defaultFactory, alternateFactories);
     }
 
     public MultiServerConnectionFactory(@Nonnull final ConnectionSelectionPolicy connectionSelectionPolicy,
                                         final int initialConnection,
                                         @Nonnull final YamlRunner.YamlConnectionFactory defaultFactory,
-                                        @Nullable final List<YamlRunner.YamlConnectionFactory> alternateFactories) {
+                                        @Nonnull final List<YamlRunner.YamlConnectionFactory> alternateFactories) {
         this.connectionSelectionPolicy = connectionSelectionPolicy;
         this.initialConnection = initialConnection;
         this.defaultFactory = defaultFactory;
         this.alternateFactories = alternateFactories;
+        this.versionsUnderTest =
+                Stream.concat(Stream.of(defaultFactory), alternateFactories.stream())
+                        .flatMap(yamlConnectionFactory -> yamlConnectionFactory.getVersionsUnderTest().stream())
+                        .collect(Collectors.toSet());
     }
 
     @Override
     public RelationalConnection getNewConnection(@Nonnull URI connectPath) throws SQLException {
         return new MultiServerRelationalConnection(connectionSelectionPolicy, initialConnection, defaultFactory.getNewConnection(connectPath), alternateConnections(connectPath));
     }
 
-    @Nullable
+    @Override
+    public Set<String> getVersionsUnderTest() {
+        return versionsUnderTest;
+    }
+
+    @Nonnull
     private List<RelationalConnection> alternateConnections(URI connectPath) {
-        if (alternateFactories == null) {
-            return null;
-        } else {
-            return alternateFactories.stream().map(factory -> {
-                try {
-                    return factory.getNewConnection(connectPath);
-                } catch (SQLException e) {
-                    throw new IllegalStateException("Failed to create a connection", e);
-                }
-            }).collect(Collectors.toList());
-        }
+        return alternateFactories.stream().map(factory -> {
+            try {
+                return factory.getNewConnection(connectPath);
+            } catch (SQLException e) {
+                throw new IllegalStateException("Failed to create a connection", e);
+            }
+        }).collect(Collectors.toList());
     }
 
     /**
@@ -115,15 +122,13 @@ public static class MultiServerRelationalConnection implements RelationalConnect
         public MultiServerRelationalConnection(@Nonnull ConnectionSelectionPolicy connectionSelectionPolicy,
                                              final int initialConnecion,
                                              @Nonnull final RelationalConnection defaultConnection,
-                                             @Nullable List<RelationalConnection> alternateConnections) {
+                                             @Nonnull List<RelationalConnection> alternateConnections) {
             this.connectionSelectionPolicy = connectionSelectionPolicy;
             this.currentConnectionSelector = initialConnecion;
             allConnections = new ArrayList<>();
             // The default connection is always the one at location 0
             allConnections.add(defaultConnection);
-            if (alternateConnections != null) {
-                allConnections.addAll(alternateConnections);
-            }
+            allConnections.addAll(alternateConnections);
         }
 
         @Override

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

@@ -28,8 +28,6 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -40,6 +38,9 @@
 import java.util.Optional;
 import java.util.function.Supplier;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
 @SuppressWarnings({"PMD.GuardLogStatement"}) // It already is, but PMD is confused and reporting error in unrelated locations.
 public final class YamlExecutionContext {
 

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

@@ -47,6 +47,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
 @SuppressWarnings({"PMD.GuardLogStatement"}) // It already is, but PMD is confused and reporting error in unrelated locations.
 public final class YamlRunner {
@@ -65,7 +66,25 @@ public final class YamlRunner {
     private final YamlExecutionContext executionContext;
 
     public interface YamlConnectionFactory {
+        /**
+         * Convert a connection uri into an actual connection.
+         * @param connectPath the path to connect to
+         * @return A new {@link RelationalConnection} for the given path appropriate for this test class
+         * @throws SQLException if we cannot connect
+         */
         RelationalConnection getNewConnection(@Nonnull URI connectPath) throws SQLException;
+
+        /**
+         * The versions that the connection has, other than the current code.
+         * <p>
+         *     If we are just testing against the current code, this will be empty, but otherwise it will include the
+         *     versions that we're testing. In the future we may want to support tests that don't run against the
+         *     current version, but that's not currently needed, so not supported.
+         * </p>
+         * @return A set of versions that we are testing against, or an empty set if just testing against the current
+         * version
+         */
+        Set<String> getVersionsUnderTest();
     }
 
     public YamlRunner(@Nonnull String resourcePath, @Nonnull YamlConnectionFactory factory, boolean correctExplain) throws RelationalException {

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

@@ -20,98 +20,36 @@
 
 package com.apple.foundationdb.relational.yamltests.block;
 
-import com.apple.foundationdb.relational.api.RelationalConnection;
 import com.apple.foundationdb.relational.util.Assert;
 import com.apple.foundationdb.relational.yamltests.CustomYamlConstructor;
 import com.apple.foundationdb.relational.yamltests.Matchers;
 import com.apple.foundationdb.relational.yamltests.YamlExecutionContext;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-
 import javax.annotation.Nonnull;
-import java.net.URI;
-import java.sql.SQLException;
-import java.util.Collection;
-import java.util.List;
-import java.util.function.Consumer;
 
 /**
- * Block is a single region in the YAMSQL file that can either be a {@link SetupBlock} or {@link TestBlock}.
+ * Block is a single region in the YAMSQL file that can either be a
+ * {@link FileOptions}, {@link SetupBlock} or {@link TestBlock}.
  * <ul>
+ *      <li> {@link FileOptions}: Controls whether a file should be run at all, and other configuration that runs before
+ *          creating the connection.</li>
  *      <li> {@link SetupBlock}: It can be either a `setup` block or a `destruct` block. The motive of these block
  *          is to "setup" and "clean" the environment needed to run the `test-block`s. A Setup block consist of a list
  *          of commands.</li>
  *      <li> {@link TestBlock}: Defines a scope for a group of tests by setting the knobs that determines how those
  *          tests are run.</li>
  * </ul>
- * <p>
- * Each block needs to be associated with a `connectionURI` which provides it with the address to the database to which
- * the block connects to for running its executables. The block does so through the
- * {@link com.apple.foundationdb.relational.yamltests.YamlRunner.YamlConnectionFactory}. A block is free to implement `how` and `when`
- * it wants to use the factory to create a connection and also manages the lifecycle of the established connection.
  */
-@SuppressWarnings({"PMD.GuardLogStatement"})
-public abstract class Block {
-
-    private static final Logger logger = LogManager.getLogger(Block.class);
-
-    static final String BLOCK_CONNECT = "connect";
-
-    int lineNumber;
-    @Nonnull
-    YamlExecutionContext executionContext;
-    @Nonnull
-    private final URI connectionURI;
-    @Nonnull
-    final List<Consumer<RelationalConnection>> executables;
-
-    Block(int lineNumber, @Nonnull List<Consumer<RelationalConnection>> executables, @Nonnull URI connectionURI, @Nonnull YamlExecutionContext executionContext) {
-        this.lineNumber = lineNumber;
-        this.executables = executables;
-        this.connectionURI = connectionURI;
-        this.executionContext = executionContext;
-    }
-
-    public int getLineNumber() {
-        return lineNumber;
-    }
-
-    /**
-     * Executes the executables from the parsed block in a single connection.
-     */
-    public abstract void execute();
-
-    protected final void executeExecutables(@Nonnull Collection<Consumer<RelationalConnection>> list) {
-        connectToDatabaseAndExecute(connection -> list.forEach(t -> t.accept(connection)));
-    }
-
-    /**
-     * Tries connecting to a database and execute the consumer using the established connection.
-     *
-     * @param consumer operations to be performed on the database using the established connection.
-     */
-    void connectToDatabaseAndExecute(Consumer<RelationalConnection> consumer) {
-        logger.debug("🚠 Connecting to database: `{}`", connectionURI);
-        try (var connection = executionContext.getConnectionFactory().getNewConnection(connectionURI)) {
-            logger.debug("✅ Connected to database: `{}`", connectionURI);
-            consumer.accept(connection);
-        } catch (SQLException sqle) {
-            throw executionContext.wrapContext(sqle,
-                    () -> String.format("‼️ Error connecting to the database `%s` in block at line %d", connectionURI, lineNumber),
-                    "connection [" + connectionURI + "]", getLineNumber());
-        }
-    }
-
+public interface Block {
     /**
      * Looks at the block to determine if its one of the valid blocks. If it is a valid one, parses it to that. This
      * method dispatches the execution to the right block which takes care of reading from the block and initializing
-     * the correctly configured {@link Block} for execution.
+     * the correctly configured {@link ConnectedBlock} for execution.
      *
      * @param document a region in the file
      * @param executionContext information needed to carry out the execution
      */
-    public static Block parse(@Nonnull Object document, @Nonnull YamlExecutionContext executionContext) {
+    static Block parse(@Nonnull Object document, @Nonnull YamlExecutionContext executionContext) {
         final var blockObject = Matchers.map(document, "block");
         Assert.thatUnchecked(blockObject.size() == 1, "Illegal Format: A block is expected to be a map of size 1");
         final var entry = Matchers.firstEntry(blockObject, "block key-value");
@@ -124,8 +62,17 @@ public static Block parse(@Nonnull Object document, @Nonnull YamlExecutionContex
                 return TestBlock.parse(lineNumber, entry.getValue(), executionContext);
             case SetupBlock.SchemaTemplateBlock.SCHEMA_TEMPLATE_BLOCK:
                 return SetupBlock.SchemaTemplateBlock.parse(lineNumber, entry.getValue(), executionContext);
+            case FileOptions.OPTIONS:
+                return FileOptions.parse(lineNumber, entry.getValue(), executionContext);
             default:
                 throw new RuntimeException("Cannot recognize the type of block");
         }
     }
+
+    int getLineNumber();
+
+    /**
+     * Executes the executables from the parsed block in a single connection.
+     */
+    void execute();
 }