address comments

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/visitors/ExpressionVisitor.java

@@ -768,9 +768,12 @@ private Expressions parseRecordFieldsUnderReorderings(@Nonnull final List<? exte
             for (final var elementField : elementFields) {
                 final int index = targetTypeReorderings.indexOf(elementField.getFieldName());
                 final var fieldType = elementField.getFieldType();
-                final Expression currentFieldColumns;
+                Expression currentFieldColumns = null;
                 if (index >= 0 && index < providedColumnContexts.size()) {
                     currentFieldColumns = parseRecordField(providedColumnContexts.get(index), elementField);
+                } else if (index >= providedColumnContexts.size()) {
+                    // column is declared but the value is not provided
+                    Assert.failUnchecked(ErrorCode.SYNTAX_ERROR, "Value of column \"" + elementField.getFieldName() + "\" is not provided");
                 } else {
                     // We do not yet support default values for any types, hence it makes to simply fail if the field type
                     // expects non-null but no value is provided.

fdb-relational-core/src/test/java/com/apple/foundationdb/relational/api/ddl/DdlStatementParsingTest.java

@@ -64,6 +64,7 @@
 import javax.annotation.Nullable;
 import java.net.URI;
 import java.sql.SQLException;
+import java.sql.Types;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -254,6 +255,75 @@ public ConstantAction getCreateSchemaTemplateConstantAction(@Nonnull SchemaTempl
         });
     }
 
+    private static Stream<Arguments> typesMap() {
+        return Stream.of(
+                Arguments.of(Types.INTEGER, "INTEGER"),
+                Arguments.of(Types.BIGINT, "BIGINT"),
+                Arguments.of(Types.FLOAT, "FLOAT"),
+                Arguments.of(Types.DOUBLE, "DOUBLE"),
+                Arguments.of(Types.VARCHAR, "STRING"),
+                Arguments.of(Types.BOOLEAN, "BOOLEAN"),
+                Arguments.of(Types.BINARY, "BYTES"),
+                Arguments.of(Types.STRUCT, "BAZ"),
+                Arguments.of(Types.ARRAY, "STRING ARRAY")
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("typesMap")
+    void columnTypeWithNull(int sqlType, @Nonnull String sqlTypeName) throws Exception {
+        final String stmt = "CREATE SCHEMA TEMPLATE test_template " +
+                "CREATE TYPE AS STRUCT baz (a bigint, b bigint) " +
+                "CREATE TABLE bar (id bigint, foo_field " + sqlTypeName + " null, PRIMARY KEY(id))";
+        shouldWorkWithInjectedFactory(stmt, new AbstractMetadataOperationsFactory() {
+            @Nonnull
+            @Override
+            public ConstantAction getCreateSchemaTemplateConstantAction(@Nonnull final SchemaTemplate template, @Nonnull final Options templateProperties) {
+                checkColumnNullability(template, sqlType, true);
+                return txn -> {
+                };
+            }
+        });
+    }
+
+    @ParameterizedTest
+    @MethodSource("typesMap")
+    void columnTypeWithNotNull(int sqlType, @Nonnull String sqlTypeName) throws Exception {
+        final String stmt = "CREATE SCHEMA TEMPLATE test_template " +
+                "CREATE TYPE AS STRUCT baz (a bigint, b bigint) " +
+                "CREATE TABLE bar (id bigint, foo_field " + sqlTypeName + " not null, PRIMARY KEY(id))";
+        if (sqlType == Types.ARRAY) {
+            shouldWorkWithInjectedFactory(stmt, new AbstractMetadataOperationsFactory() {
+                @Nonnull
+                @Override
+                public ConstantAction getCreateSchemaTemplateConstantAction(@Nonnull final SchemaTemplate template, @Nonnull final Options templateProperties) {
+                    checkColumnNullability(template, sqlType, false);
+                    return txn -> {
+                    };
+                }
+            });
+        } else {
+            shouldFailWith(stmt, ErrorCode.UNSUPPORTED_OPERATION);
+        }
+    }
+
+    private static void checkColumnNullability(@Nonnull final SchemaTemplate template, int sqlType, boolean isNullable) {
+        Assertions.assertInstanceOf(RecordLayerSchemaTemplate.class, template);
+        Assertions.assertEquals(1, ((RecordLayerSchemaTemplate) template).getTables().size(), "should have only 1 table");
+        final var table = ((RecordLayerSchemaTemplate) template).findTableByName("BAR");
+        Assertions.assertTrue(table.isPresent());
+        final var columns = table.get().getColumns();
+        Assertions.assertEquals(2, columns.size());
+        final var maybeNullableArrayColumn = columns.stream().filter(c -> c.getName().equals("FOO_FIELD")).findFirst();
+        Assertions.assertTrue(maybeNullableArrayColumn.isPresent());
+        if (isNullable) {
+            Assertions.assertTrue(maybeNullableArrayColumn.get().getDatatype().isNullable());
+        } else {
+            Assertions.assertFalse(maybeNullableArrayColumn.get().getDatatype().isNullable());
+        }
+        Assertions.assertEquals(sqlType, maybeNullableArrayColumn.get().getDatatype().getJdbcSqlCode());
+    }
+
     @Test
     void failsToParseEmptyTemplateStatements() throws Exception {
         //empty template statements are invalid, and can be rejected in the parser

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

@@ -24,10 +24,10 @@
 import com.apple.foundationdb.relational.api.EmbeddedRelationalStruct;
 import com.apple.foundationdb.relational.api.RelationalPreparedStatement;
 import com.apple.foundationdb.relational.api.RelationalStruct;
+import com.apple.foundationdb.relational.utils.RelationalAssertions;
 import com.apple.foundationdb.relational.utils.RelationalStructAssert;
 import com.apple.foundationdb.relational.utils.ResultSetAssert;
 import com.apple.foundationdb.relational.utils.SimpleDatabaseRule;
-import org.junit.Assert;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
@@ -71,7 +71,7 @@ public class RelationalArrayTest {
             "double_null double array, double_not_null double array not null, " +
             "string_null string array, string_not_null string array not null, " +
             "bytes_null bytes array, bytes_not_null bytes array not null, " +
-            "structure_null structure array, structure_not_null structure array not null, " +
+            "struct_null structure array, struct_not_null structure array not null, " +
             // "enumeration_null enumeration array, enumeration_not_null enumeration array not null, " +
             "primary key(pk))";
 
@@ -113,8 +113,7 @@ private void insertArraysViaQuerySimpleStatement() throws SQLException {
                 "null, [x'31', x'32'], " +
                 "null, [(11, '11'), (22, '22')] " +
                 ")");
-        Assert.assertThrows(SQLException.class,
-                () -> insertQuery("INSERT INTO T VALUES (" +
+        RelationalAssertions.assertThrowsSqlException(() -> insertQuery("INSERT INTO T VALUES (" +
                         "3," +
                         "[true, false], null, " +
                         "[11, 22], null, " +
@@ -125,14 +124,14 @@ private void insertArraysViaQuerySimpleStatement() throws SQLException {
                         "[x'31', x'32'], null, " +
                         "[(11, '11'), (22, '22')], null " +
                         ")"));
-        Assert.assertThrows(SQLException.class, () -> insertQuery("INSERT INTO T (pk) VALUES (4)"));
+        RelationalAssertions.assertThrowsSqlException(() -> insertQuery("INSERT INTO T (pk) VALUES (4)"));
     }
 
     @Test
     void testInsertArraysViaQueryPreparedStatement() throws SQLException {
         final var statement = "INSERT INTO T (pk, boolean_null, boolean_not_null, integer_null, integer_not_null, " +
                 "bigint_null, bigint_not_null, float_null, float_not_null, double_null, double_not_null, string_null, " +
-                "string_not_null, bytes_null, bytes_not_null, structure_null, structure_not_null) VALUES (?pk, ?boolean_null, " +
+                "string_not_null, bytes_null, bytes_not_null, struct_null, struct_not_null) VALUES (?pk, ?boolean_null, " +
                 "?boolean_not_null, ?integer_null, ?integer_not_null, ?bigint_null, ?bigint_not_null, ?float_null, " +
                 "?float_not_null, ?double_null, ?double_not_null, ?string_null, ?string_not_null, ?bytes_null, " +
                 "?bytes_not_null, ?struct_null, ?struct_not_null)";
@@ -208,7 +207,7 @@ void testInsertArraysViaQueryPreparedStatement() throws SQLException {
                 ps.setNull("bytes_not_null", Types.ARRAY);
                 ps.setArray("struct_null", EmbeddedRelationalArray.newBuilder().addAll(EmbeddedRelationalStruct.newBuilder().addInt("a", 11).addString("b", "11").build(), EmbeddedRelationalStruct.newBuilder().addInt("a", 22).addString("b", "22").build()).build());
                 ps.setNull("struct_not_null", Types.ARRAY);
-                Assert.assertThrows(SQLException.class, ps::executeUpdate);
+                RelationalAssertions.assertThrowsSqlException(ps::executeUpdate);
             }
         }
     }
@@ -222,8 +221,7 @@ void testMoreParametersThanColumns() throws SQLException {
                 ps.setInt("pk", 1);
                 ps.setArray("boolean_null", EmbeddedRelationalArray.newBuilder().addAll(true, false).build());
                 ps.setArray("boolean_not_null", EmbeddedRelationalArray.newBuilder().addAll(true, false).build());
-                final var actualMessage = Assertions.assertThrows(SQLException.class, ps::executeUpdate).getMessage();
-                assertTrue(actualMessage.contains("Too many parameters"));
+                RelationalAssertions.assertThrowsSqlException(ps::executeUpdate).containsInMessage("Too many parameters");
             }
         }
     }
@@ -235,9 +233,7 @@ void testNonNullViolation() throws SQLException {
             conn.setAutoCommit(true);
             try (final var ps = ((RelationalPreparedStatement) conn.prepareStatement("INSERT INTO T (pk) VALUES (?pk)"))) {
                 ps.setInt("pk", 1);
-                final var actualMessage = Assertions.assertThrows(SQLException.class, ps::executeUpdate).getMessage();
-                System.out.println(actualMessage);
-                assertTrue(actualMessage.contains("violates not-null constraint"));
+                RelationalAssertions.assertThrowsSqlException(ps::executeUpdate).containsInMessage("violates not-null constraint");
             }
         }
     }
@@ -249,7 +245,7 @@ void testNullFieldsGetAutomaticallyFilled() throws SQLException {
             conn.setAutoCommit(true);
             try (final var ps = ((RelationalPreparedStatement) conn.prepareStatement("INSERT INTO T (pk, boolean_not_null, " +
                     "integer_not_null, bigint_not_null, float_not_null, double_not_null, string_not_null, bytes_not_null, " +
-                    "structure_not_null) VALUES (?pk, ?boolean_not_null, ?integer_not_null, ?bigint_not_null, ?float_not_null, " +
+                    "struct_not_null) VALUES (?pk, ?boolean_not_null, ?integer_not_null, ?bigint_not_null, ?float_not_null, " +
                     "?double_not_null, ?string_not_null, ?bytes_not_null, ?struct_not_null)"))) {
                 ps.setInt("pk", 2);
                 ps.setArray("boolean_not_null", EmbeddedRelationalArray.newBuilder().addAll(true, false).build());
@@ -273,42 +269,19 @@ void testNullFieldsGetAutomaticallyFilled() throws SQLException {
                         .hasColumn("double_null", null)
                         .hasColumn("string_null", null)
                         .hasColumn("bytes_null", null)
-                        .hasColumn("structure_null", null);
+                        .hasColumn("struct_null", null);
             }
         }
     }
 
     @Test
-    void testNullColumnDoesNotRequireAValueIfNotGiven() throws SQLException {
+    void testColumnRequiresValue() throws SQLException {
         try (final var conn = DriverManager.getConnection(database.getConnectionUri().toString())) {
             conn.setSchema(database.getSchemaName());
             conn.setAutoCommit(true);
             try (final var ps = ((RelationalPreparedStatement) conn.prepareStatement("INSERT INTO T (pk, boolean_not_null, " +
                     "integer_not_null, bigint_not_null, float_not_null, double_not_null, string_not_null, bytes_not_null, " +
-                    "structure_not_null, boolean_null, integer_null) VALUES (?pk, ?boolean_not_null, ?integer_not_null, " +
-                    "?bigint_not_null, ?float_not_null, ?double_not_null, ?string_not_null, ?bytes_not_null, ?struct_not_null)"))) {
-                ps.setInt("pk", 2);
-                ps.setArray("boolean_not_null", EmbeddedRelationalArray.newBuilder().addAll(true, false).build());
-                ps.setArray("integer_not_null", EmbeddedRelationalArray.newBuilder().addAll(11, 22).build());
-                ps.setArray("bigint_not_null", EmbeddedRelationalArray.newBuilder().addAll(11L, 22L).build());
-                ps.setArray("float_not_null", EmbeddedRelationalArray.newBuilder().addAll(11.0f, 22.0f).build());
-                ps.setArray("double_not_null", EmbeddedRelationalArray.newBuilder().addAll(11.0, 22.0).build());
-                ps.setArray("string_not_null", EmbeddedRelationalArray.newBuilder().addAll("11", "22").build());
-                ps.setArray("bytes_not_null", EmbeddedRelationalArray.newBuilder().addAll(new byte[]{49}, new byte[]{50}).build());
-                ps.setArray("struct_not_null", EmbeddedRelationalArray.newBuilder().addAll(EmbeddedRelationalStruct.newBuilder().addInt("a", 11).addString("b", "11").build(), EmbeddedRelationalStruct.newBuilder().addInt("a", 22).addString("b", "22").build()).build());
-                assertEquals(1, ps.executeUpdate());
-            }
-        }
-    }
-
-    @Test
-    void testNonNullColumnRequiresValue() throws SQLException {
-        try (final var conn = DriverManager.getConnection(database.getConnectionUri().toString())) {
-            conn.setSchema(database.getSchemaName());
-            conn.setAutoCommit(true);
-            try (final var ps = ((RelationalPreparedStatement) conn.prepareStatement("INSERT INTO T (pk, boolean_not_null, " +
-                    "integer_not_null, bigint_not_null, float_not_null, double_not_null, string_not_null, bytes_not_null, " +
-                    "structure_not_null) VALUES (?pk, ?boolean_not_null, ?integer_not_null, ?bigint_not_null, ?float_not_null, " +
+                    "struct_not_null) VALUES (?pk, ?boolean_not_null, ?integer_not_null, ?bigint_not_null, ?float_not_null, " +
                     "?double_not_null, ?string_not_null)"))) {
                 ps.setInt("pk", 2);
                 ps.setArray("boolean_not_null", EmbeddedRelationalArray.newBuilder().addAll(true, false).build());
@@ -317,8 +290,7 @@ void testNonNullColumnRequiresValue() throws SQLException {
                 ps.setArray("float_not_null", EmbeddedRelationalArray.newBuilder().addAll(11.0f, 22.0f).build());
                 ps.setArray("double_not_null", EmbeddedRelationalArray.newBuilder().addAll(11.0, 22.0).build());
                 ps.setArray("string_not_null", EmbeddedRelationalArray.newBuilder().addAll("11", "22").build());
-                final var actualMessage = Assert.assertThrows(SQLException.class, ps::executeUpdate).getMessage();
-                assertTrue(actualMessage.contains("violates not-null constraint"));
+                RelationalAssertions.assertThrowsSqlException(ps::executeUpdate).containsInMessage("not provided");
             }
         }
     }

fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/ddl/DdlRecordLayerSchemaTemplateTest.java

@@ -265,8 +265,8 @@ void typeAndEnumSameNameTest() throws RelationalException, SQLException {
     }
 
     @Test
-    void notNullTypeNotAllowedTest() throws RelationalException, SQLException {
-        String template = "CREATE SCHEMA TEMPLATE not_null_column_type " +
+    void notNullNonArrayTypeNotAllowedTest() throws RelationalException, SQLException {
+        String template = "CREATE SCHEMA TEMPLATE not_null_non_array_column_type " +
                 "CREATE TABLE foo (id bigint, foo string not null, PRIMARY KEY(id))";
 
         run(statement -> RelationalAssertions.assertThrowsSqlException(() -> statement.executeUpdate(template))

yaml-tests/src/test/resources/inserts-updates-deletes.yamsql

@@ -109,10 +109,19 @@ test_block:
       # Case where not all values are provided of A. Still works, since the columns for which no values are provided can be nullable.
       - query: insert into A(A1, A2, A3) values (4);
       - supported_version: !current_version
-      - count: 1
+      - error: "42601"
     -
       # Case when the number of values is more than the number of columns specified.
       - query: insert into A(A1, A2, A3) values (5, 6, 7, 8, 9);
       - supported_version: !current_version
       - error: "42601"
+    -
+      # Case when a nullable column's value is not provided
+      - query: insert into A(A1, A3) values (6, 7);
+      - supported_version: !current_version
+      - count: 1
+    -
+      - query: select * from A where A1 = 6
+      - supported_version: !current_version
+      - result: [{ A1: 6, A2: !null , A3: 7 }]
 ...