From 98a40fa5811819758a6a046e20e2f99b9d1eed50 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Fri, 6 Dec 2024 18:58:57 +0000 Subject: [PATCH 1/9] WIP(FIR-38126): struct type --- .../tests/PreparedStatementTest.java | 34 +++++++++++++++++++ .../jdbc/resultset/column/ColumnType.java | 11 ++++++ .../firebolt/jdbc/type/FireboltDataType.java | 3 +- .../type/FireboltDataTypeDisplayNames.java | 1 + .../jdbc/resultset/FireboltResultSetTest.java | 24 +++++++++++++ 5 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/integrationTest/java/integration/tests/PreparedStatementTest.java b/src/integrationTest/java/integration/tests/PreparedStatementTest.java index bae01c915..b8c38f412 100644 --- a/src/integrationTest/java/integration/tests/PreparedStatementTest.java +++ b/src/integrationTest/java/integration/tests/PreparedStatementTest.java @@ -423,6 +423,40 @@ void shouldInsertAndSelectGeography() throws SQLException { } } + @Test + @Tag("v2") + void shouldInsertAndSelectStruct() throws SQLException { + Car car1 = Car.builder().make("Ford").sales(12345).ts(new Timestamp(2)).d(new Date(3)).build(); + + // TODO: use prepared statement ddl for more complex type + executeStatementFromFile("/statements/statement/ddl.sql"); + try (Connection connection = createConnection()) { + + try (PreparedStatement statement = connection + .prepareStatement("INSERT INTO statement_test (id) VALUES (?)")) { + statement.setLong(1, car1.getSales()); + // statement.setString(2, car1.getMake()); + // statement.setTimestamp(3, car1.getTs()); + // statement.setDate(4, car1.getD()); + statement.executeUpdate(); + } + setParam(connection, "advanced_mode", "true"); + setParam(connection, "enable_row_selection", "true"); + try (Statement statement = connection.createStatement(); + ResultSet rs = statement + .executeQuery("SELECT statement_test FROM statement_test")) { + rs.next(); + assertEquals(FireboltDataType.STRUCT.name().toLowerCase() + "(id long null)", + rs.getMetaData().getColumnTypeName(1).toLowerCase()); + String expectedJson = String.format("{\"id\":\"%d\"}", + car1.getSales()); + assertEquals(expectedJson, rs.getString(1)); + } + } finally { + executeStatementFromFile("/statements/statement/cleanup.sql"); + } + } + private QueryResult createExpectedResult(List> expectedRows) { return QueryResult.builder().databaseName(ConnectionInfo.getInstance().getDatabase()) .tableName("prepared_statement_test") diff --git a/src/main/java/com/firebolt/jdbc/resultset/column/ColumnType.java b/src/main/java/com/firebolt/jdbc/resultset/column/ColumnType.java index 4e1d23a21..a38a81994 100644 --- a/src/main/java/com/firebolt/jdbc/resultset/column/ColumnType.java +++ b/src/main/java/com/firebolt/jdbc/resultset/column/ColumnType.java @@ -21,6 +21,7 @@ import java.util.stream.Collectors; import static com.firebolt.jdbc.type.FireboltDataType.ARRAY; +import static com.firebolt.jdbc.type.FireboltDataType.STRUCT; import static com.firebolt.jdbc.type.FireboltDataType.TUPLE; import static com.firebolt.jdbc.type.FireboltDataType.ofType; @@ -62,6 +63,8 @@ public static ColumnType of(String columnType) { innerDataTypes = getCollectionSubType(FireboltDataType.ARRAY, typeWithoutNullKeyword); } else if (isType(FireboltDataType.TUPLE, typeWithoutNullKeyword)) { innerDataTypes = getCollectionSubType(FireboltDataType.TUPLE, typeWithoutNullKeyword); + } else if (isType(FireboltDataType.STRUCT, typeWithoutNullKeyword)) { + innerDataTypes = getCollectionSubType(FireboltDataType.STRUCT, typeWithoutNullKeyword); } int typeEndIndex = getTypeEndPosition(typeWithoutNullKeyword); @@ -108,6 +111,8 @@ private static List getCollectionSubType(FireboltDataType fireboltDa if (fireboltDataType.equals(TUPLE)) { types = typeWithoutNullKeyword.split(",(?![^()]*\\))"); // Regex to split on comma and ignoring comma that are between // parenthesis + } else if (fireboltDataType.equals(STRUCT)) { + types = typeWithoutNullKeyword.split(","); } else { types = new String[] {typeWithoutNullKeyword}; } @@ -177,6 +182,8 @@ public String getCompactTypeName() { return getArrayCompactTypeName(); } else if (isTuple()) { return getTupleCompactTypeName(innerTypes); + } else if (isStruct()) { + return name; } else { return dataType.getDisplayName(); } @@ -209,6 +216,10 @@ private boolean isTuple() { return dataType.equals(TUPLE); } + private boolean isStruct() { + return dataType.equals(STRUCT); + } + public ColumnType getArrayBaseColumnType() { if (innerTypes == null || innerTypes.isEmpty()) { return null; diff --git a/src/main/java/com/firebolt/jdbc/type/FireboltDataType.java b/src/main/java/com/firebolt/jdbc/type/FireboltDataType.java index acad47600..60ff5cdab 100644 --- a/src/main/java/com/firebolt/jdbc/type/FireboltDataType.java +++ b/src/main/java/com/firebolt/jdbc/type/FireboltDataType.java @@ -42,7 +42,8 @@ public enum FireboltDataType { TUPLE(Types.OTHER, FireboltDataTypeDisplayNames.TUPLE, BaseType.OBJECT, false, true, 0, 0, 0, false,"Tuple"), BYTEA(Types.BINARY, FireboltDataTypeDisplayNames.BYTEA, BaseType.BYTEA, false, true, 0, 0, 0, false, "ByteA"), GEOGRAPHY(Types.VARCHAR, FireboltDataTypeDisplayNames.GEOGRAPHY, BaseType.TEXT, false, false, 0, 0, 0, false, - "Geography"); + "Geography"), + STRUCT(Types.VARCHAR, FireboltDataTypeDisplayNames.STRUCT, BaseType.TEXT, false, false, 0, 0, 0, false, "Struct"); private static final Map typeNameOrAliasToType; diff --git a/src/main/java/com/firebolt/jdbc/type/FireboltDataTypeDisplayNames.java b/src/main/java/com/firebolt/jdbc/type/FireboltDataTypeDisplayNames.java index 23e01b680..18f9da828 100644 --- a/src/main/java/com/firebolt/jdbc/type/FireboltDataTypeDisplayNames.java +++ b/src/main/java/com/firebolt/jdbc/type/FireboltDataTypeDisplayNames.java @@ -22,4 +22,5 @@ public class FireboltDataTypeDisplayNames { static final String TUPLE = "tuple"; static final String BYTEA = "bytea"; static final String GEOGRAPHY = "geography"; + static final String STRUCT = "struct"; } \ No newline at end of file diff --git a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java index 407a4e45a..3a78a13e8 100644 --- a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java +++ b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java @@ -1478,6 +1478,26 @@ void shouldReturnGeography() throws SQLException { assertEquals(Types.VARCHAR, resultSet.getMetaData().getColumnType(9)); } + @Test + void shouldReturnStruct() throws SQLException { + inputStream = getInputStreamWithStruct(); + resultSet = createResultSet(inputStream); + resultSet.next(); + // TODO: is this correct null handling? + assertEquals("{\"a\": N}", resultSet.getObject(3)); + assertEquals("{\"a\": N}", resultSet.getObject("an_empty_struct")); + assertEquals("{\"a\": 1}", resultSet.getObject(4)); + assertEquals("{\"a\": 1}", resultSet.getObject("a_struct")); + // Returns native JDBC type + for (int i = 3; i <= 5; i++) { + assertEquals(Types.VARCHAR, resultSet.getMetaData().getColumnType(i)); + } + + assertEquals("STRUCT(A INT NULL)", resultSet.getMetaData().getColumnTypeName(3)); + assertEquals("STRUCT(A INT)", resultSet.getMetaData().getColumnTypeName(4)); + assertEquals("STRUCT(A INT)", resultSet.getMetaData().getColumnTypeName(5)); + } + @Test void shouldBeCaseInsensitive() throws SQLException { inputStream = getInputStreamWithCommonResponseExample(); @@ -1552,6 +1572,10 @@ private InputStream getInputStreamWithArray() { return FireboltResultSetTest.class.getResourceAsStream("/responses/firebolt-response-with-array"); } + private InputStream getInputStreamWithStruct() { + return FireboltResultSetTest.class.getResourceAsStream("/responses/firebolt-response-with-struct-nofalse"); + } + private ResultSet createResultSet(InputStream is) throws SQLException { return new FireboltResultSet(is, "a_table", "a_db", 65535, false, fireboltStatement, true); } From 904ca57cea0728837450f9f726a67d5ac05699ff Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 9 Dec 2024 16:41:11 +0000 Subject: [PATCH 2/9] fix build --- .../jdbc/resultset/FireboltResultSetTest.java | 14 +++++++------- .../firebolt-response-with-struct-nofalse | 4 ++++ 2 files changed, 11 insertions(+), 7 deletions(-) create mode 100644 src/test/resources/responses/firebolt-response-with-struct-nofalse diff --git a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java index 3a78a13e8..964c9b011 100644 --- a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java +++ b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java @@ -1484,18 +1484,18 @@ void shouldReturnStruct() throws SQLException { resultSet = createResultSet(inputStream); resultSet.next(); // TODO: is this correct null handling? - assertEquals("{\"a\": N}", resultSet.getObject(3)); - assertEquals("{\"a\": N}", resultSet.getObject("an_empty_struct")); - assertEquals("{\"a\": 1}", resultSet.getObject(4)); - assertEquals("{\"a\": 1}", resultSet.getObject("a_struct")); + assertEquals("{\"a\":N}", resultSet.getObject(2)); + assertEquals("{\"a\":N}", resultSet.getObject("null_struct")); + assertEquals("{\"a\":\"1\"}", resultSet.getObject(4)); + assertEquals("{\"a\":\"1\"}", resultSet.getObject("a_struct")); // Returns native JDBC type - for (int i = 3; i <= 5; i++) { + for (int i = 2; i <= 4; i++) { assertEquals(Types.VARCHAR, resultSet.getMetaData().getColumnType(i)); } - assertEquals("STRUCT(A INT NULL)", resultSet.getMetaData().getColumnTypeName(3)); + assertEquals("STRUCT(A INT NULL)", resultSet.getMetaData().getColumnTypeName(2)); + assertEquals("STRUCT(A INT)", resultSet.getMetaData().getColumnTypeName(3)); assertEquals("STRUCT(A INT)", resultSet.getMetaData().getColumnTypeName(4)); - assertEquals("STRUCT(A INT)", resultSet.getMetaData().getColumnTypeName(5)); } @Test diff --git a/src/test/resources/responses/firebolt-response-with-struct-nofalse b/src/test/resources/responses/firebolt-response-with-struct-nofalse new file mode 100644 index 000000000..06613b7c9 --- /dev/null +++ b/src/test/resources/responses/firebolt-response-with-struct-nofalse @@ -0,0 +1,4 @@ +id null_struct an_empty_struct a_struct +Int64 struct(a int null) struct(a int) struct(a int) +1 {"a":\N} {"a":"1"} +2 {"a":\N} {"a":"2"} \ No newline at end of file From 3b982f1e07d57e4f06faea395a40b0064a5cafe2 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 9 Dec 2024 17:31:15 +0000 Subject: [PATCH 3/9] verify null --- .../java/integration/tests/PreparedStatementTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/integrationTest/java/integration/tests/PreparedStatementTest.java b/src/integrationTest/java/integration/tests/PreparedStatementTest.java index b8c38f412..0dc71cf3c 100644 --- a/src/integrationTest/java/integration/tests/PreparedStatementTest.java +++ b/src/integrationTest/java/integration/tests/PreparedStatementTest.java @@ -433,7 +433,7 @@ void shouldInsertAndSelectStruct() throws SQLException { try (Connection connection = createConnection()) { try (PreparedStatement statement = connection - .prepareStatement("INSERT INTO statement_test (id) VALUES (?)")) { + .prepareStatement("INSERT INTO statement_test (id) VALUES (?),(null)")) { statement.setLong(1, car1.getSales()); // statement.setString(2, car1.getMake()); // statement.setTimestamp(3, car1.getTs()); @@ -451,6 +451,12 @@ void shouldInsertAndSelectStruct() throws SQLException { String expectedJson = String.format("{\"id\":\"%d\"}", car1.getSales()); assertEquals(expectedJson, rs.getString(1)); + // Verify null value + rs.next(); + assertEquals(FireboltDataType.STRUCT.name().toLowerCase() + "(id long null)", + rs.getMetaData().getColumnTypeName(1).toLowerCase()); + expectedJson = "{\"id\":null}"; + assertEquals(expectedJson, rs.getString(1)); } } finally { executeStatementFromFile("/statements/statement/cleanup.sql"); From b6d205c8983a57d3d6c000e7dd5290fa1293fe8b Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 9 Dec 2024 17:38:04 +0000 Subject: [PATCH 4/9] fixed integration test --- .../tests/PreparedStatementTest.java | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/integrationTest/java/integration/tests/PreparedStatementTest.java b/src/integrationTest/java/integration/tests/PreparedStatementTest.java index 0dc71cf3c..b9d400e90 100644 --- a/src/integrationTest/java/integration/tests/PreparedStatementTest.java +++ b/src/integrationTest/java/integration/tests/PreparedStatementTest.java @@ -428,38 +428,32 @@ void shouldInsertAndSelectGeography() throws SQLException { void shouldInsertAndSelectStruct() throws SQLException { Car car1 = Car.builder().make("Ford").sales(12345).ts(new Timestamp(2)).d(new Date(3)).build(); - // TODO: use prepared statement ddl for more complex type - executeStatementFromFile("/statements/statement/ddl.sql"); + executeStatementFromFile("/statements/prepared-statement/ddl.sql"); try (Connection connection = createConnection()) { try (PreparedStatement statement = connection - .prepareStatement("INSERT INTO statement_test (id) VALUES (?),(null)")) { + .prepareStatement("INSERT INTO prepared_statement_test (sales, make, ts, d) VALUES (?,?,?,?)")) { statement.setLong(1, car1.getSales()); - // statement.setString(2, car1.getMake()); - // statement.setTimestamp(3, car1.getTs()); - // statement.setDate(4, car1.getD()); + statement.setString(2, car1.getMake()); + statement.setTimestamp(3, car1.getTs()); + statement.setDate(4, car1.getD()); statement.executeUpdate(); } setParam(connection, "advanced_mode", "true"); setParam(connection, "enable_row_selection", "true"); try (Statement statement = connection.createStatement(); ResultSet rs = statement - .executeQuery("SELECT statement_test FROM statement_test")) { - rs.next(); - assertEquals(FireboltDataType.STRUCT.name().toLowerCase() + "(id long null)", - rs.getMetaData().getColumnTypeName(1).toLowerCase()); - String expectedJson = String.format("{\"id\":\"%d\"}", - car1.getSales()); - assertEquals(expectedJson, rs.getString(1)); - // Verify null value + .executeQuery("SELECT prepared_statement_test FROM prepared_statement_test")) { rs.next(); - assertEquals(FireboltDataType.STRUCT.name().toLowerCase() + "(id long null)", + assertEquals(FireboltDataType.STRUCT.name().toLowerCase() + + "(make text, sales long, ts timestamp null, d date null, signature bytea null, url text null)", rs.getMetaData().getColumnTypeName(1).toLowerCase()); - expectedJson = "{\"id\":null}"; + String expectedJson = String.format("{\"make\":\"%s\",\"sales\":\"%d\",\"ts\":\"%s\",\"d\":\"%s\",\"signature\":null,\"url\":null}", + car1.getMake(), car1.getSales(), car1.getTs().toString(), car1.getD().toString()); assertEquals(expectedJson, rs.getString(1)); } } finally { - executeStatementFromFile("/statements/statement/cleanup.sql"); + executeStatementFromFile("/statements/prepared-statement/cleanup.sql"); } } From b0f11788a6c1a4874a6837f9f217825d7b2d4d4a Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 9 Dec 2024 17:41:42 +0000 Subject: [PATCH 5/9] fix null handling in unit tests --- .../com/firebolt/jdbc/resultset/FireboltResultSetTest.java | 5 ++--- .../responses/firebolt-response-with-struct-nofalse | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java index 964c9b011..67c5ca227 100644 --- a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java +++ b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java @@ -1483,9 +1483,8 @@ void shouldReturnStruct() throws SQLException { inputStream = getInputStreamWithStruct(); resultSet = createResultSet(inputStream); resultSet.next(); - // TODO: is this correct null handling? - assertEquals("{\"a\":N}", resultSet.getObject(2)); - assertEquals("{\"a\":N}", resultSet.getObject("null_struct")); + assertEquals("{\"a\":null}", resultSet.getObject(2)); + assertEquals("{\"a\":null}", resultSet.getObject("null_struct")); assertEquals("{\"a\":\"1\"}", resultSet.getObject(4)); assertEquals("{\"a\":\"1\"}", resultSet.getObject("a_struct")); // Returns native JDBC type diff --git a/src/test/resources/responses/firebolt-response-with-struct-nofalse b/src/test/resources/responses/firebolt-response-with-struct-nofalse index 06613b7c9..841a7c64a 100644 --- a/src/test/resources/responses/firebolt-response-with-struct-nofalse +++ b/src/test/resources/responses/firebolt-response-with-struct-nofalse @@ -1,4 +1,4 @@ id null_struct an_empty_struct a_struct Int64 struct(a int null) struct(a int) struct(a int) -1 {"a":\N} {"a":"1"} -2 {"a":\N} {"a":"2"} \ No newline at end of file +1 {"a":null} {"a":"1"} +2 {"a":null} {"a":"2"} \ No newline at end of file From 8cb6f6274eb7e88feecd96e82c658f715f05ba5c Mon Sep 17 00:00:00 2001 From: ptiurin Date: Wed, 11 Dec 2024 17:05:22 +0000 Subject: [PATCH 6/9] Add complex struct test case --- .../tests/PreparedStatementTest.java | 39 +++++++++++++++++++ .../prepared-statement-struct/cleanup.sql | 2 + .../prepared-statement-struct/ddl.sql | 10 +++++ 3 files changed, 51 insertions(+) create mode 100644 src/integrationTest/resources/statements/prepared-statement-struct/cleanup.sql create mode 100644 src/integrationTest/resources/statements/prepared-statement-struct/ddl.sql diff --git a/src/integrationTest/java/integration/tests/PreparedStatementTest.java b/src/integrationTest/java/integration/tests/PreparedStatementTest.java index b9d400e90..cf3fe02fc 100644 --- a/src/integrationTest/java/integration/tests/PreparedStatementTest.java +++ b/src/integrationTest/java/integration/tests/PreparedStatementTest.java @@ -457,6 +457,44 @@ void shouldInsertAndSelectStruct() throws SQLException { } } + @Test + @Tag("v2") + void shouldInsertAndSelectComplexStruct() throws SQLException { + Car car1 = Car.builder().ts(new Timestamp(2)).d(new Date(3)).tags(new String[] { "fast", "sleek" }).build(); + + executeStatementFromFile("/statements/prepared-statement-struct/ddl.sql"); + try (Connection connection = createConnection()) { + + try (PreparedStatement statement = connection + .prepareStatement("INSERT INTO test_struct_helper(a, b) VALUES (?,?)")) { + statement.setArray(1, connection.createArrayOf("VARCHAR", car1.getTags())); + statement.setTimestamp(2, car1.getTs()); + statement.executeUpdate(); + } + + setParam(connection, "advanced_mode", "true"); + setParam(connection, "enable_row_selection", "true"); + try (Statement statement = connection.createStatement()) { + statement.execute( + "INSERT INTO test_struct(id, s) SELECT 1, test_struct_helper FROM test_struct_helper"); + } + try (Statement statement = connection.createStatement(); + ResultSet rs = statement + .executeQuery("SELECT test_struct FROM test_struct")) { + rs.next(); + assertEquals(FireboltDataType.STRUCT.name().toLowerCase() + + "(id int, s struct(a array(text null), b timestamp null))", + rs.getMetaData().getColumnTypeName(1).toLowerCase()); + String expectedJson = String.format( + "{\"id\":%d,\"s\":{\"a\":[\"%s\",\"%s\"],\"b\":\"%s\"}}", 1, car1.getTags()[0], + car1.getTags()[1], car1.getTs().toString()); + assertEquals(expectedJson, rs.getString(1)); + } + } finally { + executeStatementFromFile("/statements/prepared-statement-struct/cleanup.sql"); + } + } + private QueryResult createExpectedResult(List> expectedRows) { return QueryResult.builder().databaseName(ConnectionInfo.getInstance().getDatabase()) .tableName("prepared_statement_test") @@ -527,6 +565,7 @@ private static class Car { Timestamp ts; Date d; URL url; + String[] tags; } } diff --git a/src/integrationTest/resources/statements/prepared-statement-struct/cleanup.sql b/src/integrationTest/resources/statements/prepared-statement-struct/cleanup.sql new file mode 100644 index 000000000..5df6e649c --- /dev/null +++ b/src/integrationTest/resources/statements/prepared-statement-struct/cleanup.sql @@ -0,0 +1,2 @@ +DROP TABLE IF EXISTS test_struct; +DROP TABLE IF EXISTS test_struct_helper; \ No newline at end of file diff --git a/src/integrationTest/resources/statements/prepared-statement-struct/ddl.sql b/src/integrationTest/resources/statements/prepared-statement-struct/ddl.sql new file mode 100644 index 000000000..333f372a0 --- /dev/null +++ b/src/integrationTest/resources/statements/prepared-statement-struct/ddl.sql @@ -0,0 +1,10 @@ +SET advanced_mode=1; +SET enable_struct=1; +SET enable_create_table_v2=true; +SET enable_row_selection=true; +SET prevent_create_on_information_schema=true; +SET enable_create_table_with_struct_type=true; +DROP TABLE IF EXISTS test_struct; +DROP TABLE IF EXISTS test_struct_helper; +CREATE TABLE IF NOT EXISTS test_struct(id int not null, s struct(a array(text) not null, b datetime null) not null); +CREATE TABLE IF NOT EXISTS test_struct_helper(a array(text) not null, b datetime null); From 88ac7bcc743d5ee4053378329cbce6a70c4a3e16 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Wed, 11 Dec 2024 17:09:18 +0000 Subject: [PATCH 7/9] newlines --- .../resources/statements/prepared-statement-struct/cleanup.sql | 2 +- .../resources/responses/firebolt-response-with-struct-nofalse | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/integrationTest/resources/statements/prepared-statement-struct/cleanup.sql b/src/integrationTest/resources/statements/prepared-statement-struct/cleanup.sql index 5df6e649c..5465a5e55 100644 --- a/src/integrationTest/resources/statements/prepared-statement-struct/cleanup.sql +++ b/src/integrationTest/resources/statements/prepared-statement-struct/cleanup.sql @@ -1,2 +1,2 @@ DROP TABLE IF EXISTS test_struct; -DROP TABLE IF EXISTS test_struct_helper; \ No newline at end of file +DROP TABLE IF EXISTS test_struct_helper; diff --git a/src/test/resources/responses/firebolt-response-with-struct-nofalse b/src/test/resources/responses/firebolt-response-with-struct-nofalse index 841a7c64a..9212615f9 100644 --- a/src/test/resources/responses/firebolt-response-with-struct-nofalse +++ b/src/test/resources/responses/firebolt-response-with-struct-nofalse @@ -1,4 +1,4 @@ id null_struct an_empty_struct a_struct Int64 struct(a int null) struct(a int) struct(a int) 1 {"a":null} {"a":"1"} -2 {"a":null} {"a":"2"} \ No newline at end of file +2 {"a":null} {"a":"2"} From 36b7ad20d102e7492558e2a2e189f8f0c0f18395 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Thu, 12 Dec 2024 11:27:52 +0000 Subject: [PATCH 8/9] more test cases --- .../jdbc/resultset/FireboltResultSetTest.java | 69 ++++++++++--------- .../firebolt-response-with-struct-nofalse | 8 +-- 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java index 67c5ca227..833126369 100644 --- a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java +++ b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java @@ -1,18 +1,22 @@ package com.firebolt.jdbc.resultset; -import com.firebolt.jdbc.CheckedFunction; -import com.firebolt.jdbc.exception.FireboltException; -import com.firebolt.jdbc.statement.FireboltStatement; -import com.firebolt.jdbc.util.LoggerUtil; -import org.apache.commons.io.IOUtils; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.jupiter.api.function.Executable; -import org.junitpioneer.jupiter.DefaultTimeZone; -import org.mockito.Mock; -import org.mockito.MockedStatic; -import org.mockito.junit.jupiter.MockitoExtension; +import static com.firebolt.jdbc.exception.ExceptionType.TYPE_TRANSFORMATION_ERROR; +import static java.lang.String.format; +import static java.sql.ResultSet.TYPE_FORWARD_ONLY; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -48,23 +52,20 @@ import java.util.TimeZone; import java.util.concurrent.Callable; -import static com.firebolt.jdbc.exception.ExceptionType.TYPE_TRANSFORMATION_ERROR; -import static java.lang.String.format; -import static java.sql.ResultSet.TYPE_FORWARD_ONLY; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.mockStatic; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; +import org.apache.commons.io.IOUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.function.Executable; +import org.junitpioneer.jupiter.DefaultTimeZone; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.jupiter.MockitoExtension; + +import com.firebolt.jdbc.CheckedFunction; +import com.firebolt.jdbc.exception.FireboltException; +import com.firebolt.jdbc.statement.FireboltStatement; +import com.firebolt.jdbc.util.LoggerUtil; @ExtendWith(MockitoExtension.class) @DefaultTimeZone("UTC") @@ -1487,14 +1488,20 @@ void shouldReturnStruct() throws SQLException { assertEquals("{\"a\":null}", resultSet.getObject("null_struct")); assertEquals("{\"a\":\"1\"}", resultSet.getObject(4)); assertEquals("{\"a\":\"1\"}", resultSet.getObject("a_struct")); + assertEquals("{\"a\":[1,2,3]}", resultSet.getObject(5)); + assertEquals("{\"a\":[1,2,3]}", resultSet.getObject("array_struct")); + assertEquals("{\"x\":\"2\",\"a\":{\"b\":\"1\"}}", resultSet.getObject(6)); + assertEquals("{\"x\":\"2\",\"a\":{\"b\":\"1\"}}", resultSet.getObject("nested_struct")); // Returns native JDBC type - for (int i = 2; i <= 4; i++) { + for (int i = 2; i <= 6; i++) { assertEquals(Types.VARCHAR, resultSet.getMetaData().getColumnType(i)); } assertEquals("STRUCT(A INT NULL)", resultSet.getMetaData().getColumnTypeName(2)); assertEquals("STRUCT(A INT)", resultSet.getMetaData().getColumnTypeName(3)); assertEquals("STRUCT(A INT)", resultSet.getMetaData().getColumnTypeName(4)); + assertEquals("STRUCT(A ARRAY(INT))", resultSet.getMetaData().getColumnTypeName(5)); + assertEquals("STRUCT(X INT, A STRUCT(B INT))", resultSet.getMetaData().getColumnTypeName(6)); } @Test diff --git a/src/test/resources/responses/firebolt-response-with-struct-nofalse b/src/test/resources/responses/firebolt-response-with-struct-nofalse index 9212615f9..c19956faf 100644 --- a/src/test/resources/responses/firebolt-response-with-struct-nofalse +++ b/src/test/resources/responses/firebolt-response-with-struct-nofalse @@ -1,4 +1,4 @@ -id null_struct an_empty_struct a_struct -Int64 struct(a int null) struct(a int) struct(a int) -1 {"a":null} {"a":"1"} -2 {"a":null} {"a":"2"} +id null_struct an_empty_struct a_struct array_struct nested_struct +Int64 struct(a int null) struct(a int) struct(a int) struct(a array(int)) struct(x int, a struct(b int)) +1 {"a":null} {"a":"1"} {"a":[1,2,3]} {"x":"2","a":{"b":"1"}} +2 {"a":null} {"a":"2"} {"a":[1,2,3]} {"x":"2","a":{"b":"1"}} From 6b1fdc699b224dc25ad9f107cca56cc8c50b81a4 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Thu, 12 Dec 2024 13:29:45 +0000 Subject: [PATCH 9/9] Parsing complex structs --- .../com/firebolt/jdbc/resultset/column/ColumnType.java | 7 ++++--- .../com/firebolt/jdbc/resultset/FireboltResultSetTest.java | 6 +++--- .../responses/firebolt-response-with-struct-nofalse | 6 +++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/firebolt/jdbc/resultset/column/ColumnType.java b/src/main/java/com/firebolt/jdbc/resultset/column/ColumnType.java index a38a81994..cb92c02d2 100644 --- a/src/main/java/com/firebolt/jdbc/resultset/column/ColumnType.java +++ b/src/main/java/com/firebolt/jdbc/resultset/column/ColumnType.java @@ -41,6 +41,8 @@ public class ColumnType { private static final Set TIMEZONES = Arrays.stream(TimeZone.getAvailableIDs()) .collect(Collectors.toCollection(HashSet::new)); private static final Pattern COMMA_WITH_SPACES = Pattern.compile("\\s*,\\s*"); + // Regex to split on comma and ignoring commas that are between parenthesis + private static final String COMPLEX_TYPE_PATTERN = ",(?![^()]*\\))"; @EqualsAndHashCode.Exclude String name; FireboltDataType dataType; @@ -109,10 +111,9 @@ private static List getCollectionSubType(FireboltDataType fireboltDa } if (fireboltDataType.equals(TUPLE)) { - types = typeWithoutNullKeyword.split(",(?![^()]*\\))"); // Regex to split on comma and ignoring comma that are between - // parenthesis + types = typeWithoutNullKeyword.split(COMPLEX_TYPE_PATTERN); } else if (fireboltDataType.equals(STRUCT)) { - types = typeWithoutNullKeyword.split(","); + types = typeWithoutNullKeyword.split(COMPLEX_TYPE_PATTERN); } else { types = new String[] {typeWithoutNullKeyword}; } diff --git a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java index 833126369..63ae686eb 100644 --- a/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java +++ b/src/test/java/com/firebolt/jdbc/resultset/FireboltResultSetTest.java @@ -1490,8 +1490,8 @@ void shouldReturnStruct() throws SQLException { assertEquals("{\"a\":\"1\"}", resultSet.getObject("a_struct")); assertEquals("{\"a\":[1,2,3]}", resultSet.getObject(5)); assertEquals("{\"a\":[1,2,3]}", resultSet.getObject("array_struct")); - assertEquals("{\"x\":\"2\",\"a\":{\"b\":\"1\"}}", resultSet.getObject(6)); - assertEquals("{\"x\":\"2\",\"a\":{\"b\":\"1\"}}", resultSet.getObject("nested_struct")); + assertEquals("{\"x\":\"2\",\"a\":{\"b\":\"1\",\"c\":\"3\"}}", resultSet.getObject(6)); + assertEquals("{\"x\":\"2\",\"a\":{\"b\":\"1\",\"c\":\"3\"}}", resultSet.getObject("nested_struct")); // Returns native JDBC type for (int i = 2; i <= 6; i++) { assertEquals(Types.VARCHAR, resultSet.getMetaData().getColumnType(i)); @@ -1501,7 +1501,7 @@ void shouldReturnStruct() throws SQLException { assertEquals("STRUCT(A INT)", resultSet.getMetaData().getColumnTypeName(3)); assertEquals("STRUCT(A INT)", resultSet.getMetaData().getColumnTypeName(4)); assertEquals("STRUCT(A ARRAY(INT))", resultSet.getMetaData().getColumnTypeName(5)); - assertEquals("STRUCT(X INT, A STRUCT(B INT))", resultSet.getMetaData().getColumnTypeName(6)); + assertEquals("STRUCT(X INT, A STRUCT(B INT, C INT))", resultSet.getMetaData().getColumnTypeName(6)); } @Test diff --git a/src/test/resources/responses/firebolt-response-with-struct-nofalse b/src/test/resources/responses/firebolt-response-with-struct-nofalse index c19956faf..b706be027 100644 --- a/src/test/resources/responses/firebolt-response-with-struct-nofalse +++ b/src/test/resources/responses/firebolt-response-with-struct-nofalse @@ -1,4 +1,4 @@ id null_struct an_empty_struct a_struct array_struct nested_struct -Int64 struct(a int null) struct(a int) struct(a int) struct(a array(int)) struct(x int, a struct(b int)) -1 {"a":null} {"a":"1"} {"a":[1,2,3]} {"x":"2","a":{"b":"1"}} -2 {"a":null} {"a":"2"} {"a":[1,2,3]} {"x":"2","a":{"b":"1"}} +Int64 struct(a int null) struct(a int) struct(a int) struct(a array(int)) struct(x int, a struct(b int, c int)) +1 {"a":null} {"a":"1"} {"a":[1,2,3]} {"x":"2","a":{"b":"1","c":"3"}} +2 {"a":null} {"a":"2"} {"a":[1,2,3]} {"x":"2","a":{"b":"1","c":"3"}}