diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 7e7c057b55..8cef0ace16 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -23,7 +23,7 @@ Our API stability annotations have been updated to reflect greater API instabili * **Bug fix** Fix 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Bug fix** Fix 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Bug fix** Fix 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) -* **Bug fix** Fix 5 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) +* **Bug fix** Relational insert statement in does not work with an array value [(Issue #3041)](https://github.com/FoundationDB/fdb-record-layer/issues/3041) * **Performance** Improvement 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Performance** Improvement 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Performance** Improvement 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) diff --git a/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/SqlTypeNamesSupport.java b/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/SqlTypeNamesSupport.java index 1032a42e83..c2afc3f991 100644 --- a/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/SqlTypeNamesSupport.java +++ b/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/SqlTypeNamesSupport.java @@ -62,6 +62,8 @@ public static String getSqlTypeName(int sqlTypeCode) { return "NULL"; case Types.OTHER: return "OTHER"; + case Types.BOOLEAN: + return "BOOLEAN"; default: throw new IllegalStateException("Unexpected sql type code :" + sqlTypeCode); } @@ -87,6 +89,8 @@ public static int getSqlTypeCode(String sqlTypeName) { return Types.ARRAY; case "NULL": return Types.NULL; + case "BOOLEAN": + return Types.BOOLEAN; default: throw new IllegalStateException("Unexpected sql type name:" + sqlTypeName); } diff --git a/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/api/EmbeddedRelationalArray.java b/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/api/EmbeddedRelationalArray.java index 47503f8e6c..a25e6c69c1 100644 --- a/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/api/EmbeddedRelationalArray.java +++ b/fdb-relational-core/src/main/java/com/apple/foundationdb/relational/api/EmbeddedRelationalArray.java @@ -125,7 +125,7 @@ private void checkMetadata(@Nonnull StructMetaData metaData) throws SQLException Assert.that(this.metaData.getElementType() == Types.STRUCT, ErrorCode.DATATYPE_MISMATCH, "Expected array element to be of type:%s, but found type:STRUCT", this.metaData.getElementTypeName()); Assert.that(this.metaData.getElementStructMetaData().equals(metaData), ErrorCode.DATATYPE_MISMATCH, "Metadata of struct elements in array do not match!"); } else { - this.metaData = RelationalArrayMetaData.ofStruct(metaData, DatabaseMetaData.columnNullable); + this.metaData = RelationalArrayMetaData.ofStruct(metaData, DatabaseMetaData.columnNoNulls); } } @@ -133,7 +133,7 @@ private void checkMetadata(int sqlType) throws SQLException, RelationalException if (this.metaData != null) { Assert.that(this.metaData.getElementType() == sqlType, ErrorCode.DATATYPE_MISMATCH, "Expected array element to be of type:%s, but found type:%s", this.metaData.getElementTypeName(), SqlTypeNamesSupport.getSqlTypeName(sqlType)); } else { - this.metaData = RelationalArrayMetaData.ofPrimitive(sqlType, DatabaseMetaData.columnNullable); + this.metaData = RelationalArrayMetaData.ofPrimitive(sqlType, DatabaseMetaData.columnNoNulls); } } } diff --git a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/RelationalArrayTest.java b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/RelationalArrayTest.java index ab32b8af3e..d72e4d6d7e 100644 --- a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/RelationalArrayTest.java +++ b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/RelationalArrayTest.java @@ -20,14 +20,16 @@ package com.apple.foundationdb.relational.recordlayer; +import com.apple.foundationdb.relational.api.EmbeddedRelationalArray; 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.api.exceptions.RelationalException; -import com.apple.foundationdb.relational.utils.SimpleDatabaseRule; import com.apple.foundationdb.relational.utils.RelationalStructAssert; - +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; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -72,7 +74,7 @@ public class RelationalArrayTest { // "enumeration_null enumeration array, enumeration_not_null enumeration array not null, " + "primary key(pk))"; - public void insertQuery(@Nonnull String q) throws RelationalException, SQLException { + public void insertQuery(@Nonnull String q) throws SQLException { try (final var conn = DriverManager.getConnection(database.getConnectionUri().toString())) { conn.setSchema(database.getSchemaName()); conn.setAutoCommit(true); @@ -82,7 +84,12 @@ public void insertQuery(@Nonnull String q) throws RelationalException, SQLExcept } } - private void testInsertions() throws SQLException, RelationalException { + @Test + void testInsertArraysViaQuerySimpleStatement() throws SQLException { + insertArraysViaQuerySimpleStatement(); + } + + private void insertArraysViaQuerySimpleStatement() throws SQLException { insertQuery("INSERT INTO T VALUES (" + "1," + "[true, false], [true, false], " + @@ -124,6 +131,91 @@ private void testInsertions() throws SQLException, RelationalException { 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, 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)"; + + try (final var conn = DriverManager.getConnection(database.getConnectionUri().toString())) { + conn.setSchema(database.getSchemaName()); + conn.setAutoCommit(true); + try (final var ps = ((RelationalPreparedStatement) conn.prepareStatement(statement))) { + 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()); + ps.setArray("integer_null", EmbeddedRelationalArray.newBuilder().addAll(11, 22).build()); + ps.setArray("integer_not_null", EmbeddedRelationalArray.newBuilder().addAll(11, 22).build()); + ps.setArray("bigint_null", EmbeddedRelationalArray.newBuilder().addAll(11L, 22L).build()); + ps.setArray("bigint_not_null", EmbeddedRelationalArray.newBuilder().addAll(11L, 22L).build()); + ps.setArray("float_null", EmbeddedRelationalArray.newBuilder().addAll(11.0f, 22.0f).build()); + ps.setArray("float_not_null", EmbeddedRelationalArray.newBuilder().addAll(11.0f, 22.0f).build()); + ps.setArray("double_null", EmbeddedRelationalArray.newBuilder().addAll(11.0, 22.0).build()); + ps.setArray("double_not_null", EmbeddedRelationalArray.newBuilder().addAll(11.0, 22.0).build()); + ps.setArray("string_null", EmbeddedRelationalArray.newBuilder().addAll("11", "22").build()); + ps.setArray("string_not_null", EmbeddedRelationalArray.newBuilder().addAll("11", "22").build()); + ps.setArray("bytes_null", EmbeddedRelationalArray.newBuilder().addAll(new byte[]{49}, new byte[]{50}).build()); + ps.setArray("bytes_not_null", EmbeddedRelationalArray.newBuilder().addAll(new byte[]{49}, new byte[]{50}).build()); + 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.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()); + Assertions.assertEquals(1, ps.executeUpdate()); + } + } + + try (final var conn = DriverManager.getConnection(database.getConnectionUri().toString())) { + conn.setSchema(database.getSchemaName()); + conn.setAutoCommit(true); + try (final var ps = ((RelationalPreparedStatement) conn.prepareStatement(statement))) { + ps.setInt("pk", 2); + ps.setNull("boolean_null", Types.ARRAY); + ps.setArray("boolean_not_null", EmbeddedRelationalArray.newBuilder().addAll(true, false).build()); + ps.setNull("integer_null", Types.ARRAY); + ps.setArray("integer_not_null", EmbeddedRelationalArray.newBuilder().addAll(11, 22).build()); + ps.setNull("bigint_null", Types.ARRAY); + ps.setArray("bigint_not_null", EmbeddedRelationalArray.newBuilder().addAll(11L, 22L).build()); + ps.setNull("float_null", Types.ARRAY); + ps.setArray("float_not_null", EmbeddedRelationalArray.newBuilder().addAll(11.0f, 22.0f).build()); + ps.setNull("double_null", Types.ARRAY); + ps.setArray("double_not_null", EmbeddedRelationalArray.newBuilder().addAll(11.0, 22.0).build()); + ps.setNull("string_null", Types.ARRAY); + ps.setArray("string_not_null", EmbeddedRelationalArray.newBuilder().addAll("11", "22").build()); + ps.setNull("bytes_null", Types.ARRAY); + ps.setArray("bytes_not_null", EmbeddedRelationalArray.newBuilder().addAll(new byte[]{49}, new byte[]{50}).build()); + ps.setNull("struct_null", Types.ARRAY); + 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()); + Assertions.assertEquals(1, ps.executeUpdate()); + } + } + + try (final var conn = DriverManager.getConnection(database.getConnectionUri().toString())) { + conn.setSchema(database.getSchemaName()); + conn.setAutoCommit(true); + try (final var ps = ((RelationalPreparedStatement) conn.prepareStatement(statement))) { + ps.setInt("pk", 2); + ps.setArray("boolean_null", EmbeddedRelationalArray.newBuilder().addAll(true, false).build()); + ps.setNull("boolean_not_null", Types.ARRAY); + ps.setArray("integer_null", EmbeddedRelationalArray.newBuilder().addAll(11, 22).build()); + ps.setNull("integer_not_null", Types.ARRAY); + ps.setArray("bigint_null", EmbeddedRelationalArray.newBuilder().addAll(11L, 22L).build()); + ps.setNull("bigint_not_null", Types.ARRAY); + ps.setArray("float_null", EmbeddedRelationalArray.newBuilder().addAll(11.0f, 22.0f).build()); + ps.setNull("float_not_null", Types.ARRAY); + ps.setArray("double_null", EmbeddedRelationalArray.newBuilder().addAll(11.0, 22.0).build()); + ps.setNull("double_not_null", Types.ARRAY); + ps.setArray("string_null", EmbeddedRelationalArray.newBuilder().addAll("11", "22").build()); + ps.setNull("string_not_null", Types.ARRAY); + ps.setArray("bytes_null", EmbeddedRelationalArray.newBuilder().addAll(new byte[]{49}, new byte[]{50}).build()); + 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); + } + } + } + private static Stream provideTypesForArrayTests() throws SQLException { return Stream.of( Arguments.of(2, 3, List.of(true, false), Types.BOOLEAN), @@ -142,9 +234,8 @@ private static Stream provideTypesForArrayTests() throws SQLException @ParameterizedTest @MethodSource("provideTypesForArrayTests") - void testArrays(int nullArrayIdx, int nonNullArrayIdx, List filledArray, int sqlType) - throws SQLException, RelationalException { - testInsertions(); + void testArrays(int nullArrayIdx, int nonNullArrayIdx, List filledArray, int sqlType) throws SQLException { + insertArraysViaQuerySimpleStatement(); testArrays(nullArrayIdx, nonNullArrayIdx, filledArray, filledArray, sqlType, 1); testArrays(nullArrayIdx, nonNullArrayIdx, null, filledArray, sqlType, 2); testArrays(nullArrayIdx, nonNullArrayIdx, null, List.of(), sqlType, 4); @@ -181,7 +272,6 @@ private void checkArrayElementsUsingGetResultSet(@Nonnull ResultSet arrayResultS final var arrayMetadata = arrayResultSet.getMetaData(); assertEquals(2, arrayMetadata.getColumnCount()); assertEquals(ResultSetMetaData.columnNoNulls, arrayMetadata.isNullable(1)); - // TODO: TODO (Support array element nullability): The nullability of array element is not being propagated correctly! assertEquals(ResultSetMetaData.columnNoNulls, arrayMetadata.isNullable(2)); assertEquals(Types.INTEGER, arrayMetadata.getColumnType(1)); assertEquals(sqlType, arrayMetadata.getColumnType(2)); @@ -200,7 +290,7 @@ private void checkArrayElementsUsingGetResultSet(@Nonnull ResultSet arrayResultS assertFalse(arrayResultSet.next()); } - private void checkArrayElementsUsingGetArray(@Nonnull Object[] array, int sqlType, List elements) throws SQLException { + private void checkArrayElementsUsingGetArray(@Nonnull Object[] array, int sqlType, List elements) { assertEquals(elements.size(), array.length); for (int i = 0; i < elements.size(); i++) { final var object = array[i]; diff --git a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/TableWithEnumTest.java b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/TableWithEnumTest.java index 677eb8ae06..ed1e0442fd 100644 --- a/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/TableWithEnumTest.java +++ b/fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/TableWithEnumTest.java @@ -25,16 +25,14 @@ import com.apple.foundationdb.relational.api.Options; import com.apple.foundationdb.relational.api.RelationalResultSet; import com.apple.foundationdb.relational.api.RelationalStruct; -import com.apple.foundationdb.relational.api.exceptions.ContextualSQLException; import com.apple.foundationdb.relational.api.exceptions.ErrorCode; import com.apple.foundationdb.relational.util.Assert; +import com.apple.foundationdb.relational.utils.RelationalAssertions; import com.apple.foundationdb.relational.utils.ResultSetAssert; import com.apple.foundationdb.relational.utils.SimpleDatabaseRule; import com.apple.foundationdb.relational.utils.TestSchemas; -import com.apple.foundationdb.relational.utils.RelationalAssertions; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -44,7 +42,6 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.fail; public class TableWithEnumTest { @@ -69,7 +66,7 @@ public class TableWithEnumTest { public final RelationalStatementRule statement = new RelationalStatementRule(connection); @Test - void canInsertAndGetSingleRecord() throws Exception { + void canInsertViaDirectAccess() throws Exception { final var inserted = insertCard(42, "HEARTS", 8); KeySet keys = new KeySet() @@ -83,6 +80,37 @@ void canInsertAndGetSingleRecord() throws Exception { } } + @Test + void canInsertViaQuerySimpleStatement() throws SQLException { + statement.execute("INSERT INTO CARD (id, suit, rank) VALUES (1, 'HEARTS', 4)"); + final var expectedStruct = getStructToInsert(1, "HEARTS", 4); + + try (RelationalResultSet resultSet = statement.executeQuery("SELECT * FROM CARD WHERE ID = 1")) { + ResultSetAssert.assertThat(resultSet) + .hasNextRow() + .isRowExactly(expectedStruct) + .hasNoNextRow(); + } + } + + @Test + void canInsertViaQueryPreparedStatement() throws SQLException { + final var preparedStmt = connection.prepareStatement("INSERT INTO CARD (id, suit, rank) VALUES (?id, ?suit, ?rank)"); + preparedStmt.setLong("id", 1); + preparedStmt.setObject("suit", "HEARTS"); + preparedStmt.setLong("rank", 4); + final var count = preparedStmt.executeUpdate(); + assertThat(count).isEqualTo(1); + + final var expectedStruct = getStructToInsert(1, "HEARTS", 4); + try (RelationalResultSet resultSet = statement.executeQuery("SELECT * FROM CARD WHERE ID = 1")) { + ResultSetAssert.assertThat(resultSet) + .hasNextRow() + .isRowExactly(expectedStruct) + .hasNoNextRow(); + } + } + @Test void canInsertFiftyTwoCards() throws Exception { insert52Cards(); @@ -181,22 +209,6 @@ void unsetSuit() throws SQLException { } } - /** - * Inserting a value via a query is not yet supported. Once it is, however, we should validate that proper - * enum conversions are performed. Additionally, it would be good to make sure we test what happens if - * an invalid enum value is specified. - * - * (yhatem) this _almost_ now works, we just miss a STRING-to-ENUM promotion path (https://github.com/FoundationDB/fdb-record-layer/issues/1946) - * disabling until we fix this issue. - */ - @Test - @Disabled("require a record-layer fix https://github.com/FoundationDB/fdb-record-layer/issues/1946") - void insertViaQuery() { - assertThatThrownBy(() -> statement.execute("INSERT INTO Card (id, suit, rank) VALUES (1, 'HEARTS', 4)")) - .isInstanceOf(ContextualSQLException.class) - .hasMessageContaining("query is not supported"); - } - private void insert52Cards() throws Exception { connection.setAutoCommit(false); int cardId = 0;