From 9b4b735d032f12a5ddd3a58689e021927746238f Mon Sep 17 00:00:00 2001 From: ptiurin Date: Tue, 15 Oct 2024 18:38:21 +0100 Subject: [PATCH 01/13] fix: fix parameter formatting for new Firebolt --- .../tests/PreparedStatementTest.java | 18 ++++++++++++++++++ .../jdbc/type/JavaTypeToFireboltSQLString.java | 2 +- .../type/JavaTypeToFireboltSQLStringTest.java | 8 +++++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/integrationTest/java/integration/tests/PreparedStatementTest.java b/src/integrationTest/java/integration/tests/PreparedStatementTest.java index 779ab50a9..65079e66a 100644 --- a/src/integrationTest/java/integration/tests/PreparedStatementTest.java +++ b/src/integrationTest/java/integration/tests/PreparedStatementTest.java @@ -435,6 +435,24 @@ void shouldInsertAndRetrieveUrl() throws SQLException, MalformedURLException { } } + @Test + void shouldFetchSpecialCharacters() throws SQLException, MalformedURLException { + try (Connection connection = createConnection()) { + try (PreparedStatement statement = connection + .prepareStatement("SELECT ? as a, ? as b, ? as c ")) { + statement.setString(1, "\0"); + statement.setString(2, "\\"); + statement.setString(3, "don't"); + statement.execute(); + ResultSet rs = statement.getResultSet(); + assertTrue(rs.next()); + assertEquals("\0", rs.getString(1)); + assertEquals("\\", rs.getString(2)); + assertEquals("don't", rs.getString(3)); + } + } + } + @Builder @Value @EqualsAndHashCode diff --git a/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java b/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java index 2229f622a..7535d1b71 100644 --- a/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java +++ b/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java @@ -48,7 +48,7 @@ public enum JavaTypeToFireboltSQLString { BYTE_ARRAY(byte[].class, value -> ofNullable(byteArrayToHexString((byte[])value, true)).map(x -> format("E'%s'::BYTEA", x)).orElse(null)), ; private static final List> characterToEscapedCharacterPairs = List.of( - Map.entry("\0", "\\0"), Map.entry("\\", "\\\\"), Map.entry("'", "''")); + Map.entry("'", "''")); //https://docs.oracle.com/javase/1.5.0/docs/guide/jdbc/getstart/mapping.html private static final Map> jdbcTypeToClass = Map.ofEntries( Map.entry(JDBCType.CHAR, String.class), diff --git a/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java b/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java index 86c111c17..3a22fad33 100644 --- a/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java +++ b/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java @@ -69,9 +69,15 @@ void shouldTransformShortToString() throws SQLException { @Test void shouldEscapeCharactersWhenTransformingFromString() throws SQLException { + // quotes are escaped assertEquals("'105'' OR 1=1--'' '", JavaTypeToFireboltSQLString.STRING.transform("105' OR 1=1--' ")); - assertEquals("'105'' OR 1=1--'' '", JavaTypeToFireboltSQLString.transformAny("105' OR 1=1--' ")); + // \0 is not escaped + assertEquals("'105\0'", JavaTypeToFireboltSQLString.STRING.transform("105\0")); + assertEquals("'105\0'", JavaTypeToFireboltSQLString.transformAny("105\0")); + // backslashes are not escaped + assertEquals("'105\\'", JavaTypeToFireboltSQLString.STRING.transform("105\\")); + assertEquals("'105\\'", JavaTypeToFireboltSQLString.transformAny("105\\")); } @Test From 15ee5573ba263739772a28c62216109848ef0d8f Mon Sep 17 00:00:00 2001 From: ptiurin Date: Wed, 16 Oct 2024 09:46:34 +0100 Subject: [PATCH 02/13] fix test --- .../java/integration/tests/PreparedStatementTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/integrationTest/java/integration/tests/PreparedStatementTest.java b/src/integrationTest/java/integration/tests/PreparedStatementTest.java index 65079e66a..36a81626f 100644 --- a/src/integrationTest/java/integration/tests/PreparedStatementTest.java +++ b/src/integrationTest/java/integration/tests/PreparedStatementTest.java @@ -440,13 +440,13 @@ void shouldFetchSpecialCharacters() throws SQLException, MalformedURLException { try (Connection connection = createConnection()) { try (PreparedStatement statement = connection .prepareStatement("SELECT ? as a, ? as b, ? as c ")) { - statement.setString(1, "\0"); + statement.setString(1, "\\0"); statement.setString(2, "\\"); statement.setString(3, "don't"); statement.execute(); ResultSet rs = statement.getResultSet(); assertTrue(rs.next()); - assertEquals("\0", rs.getString(1)); + assertEquals("\\0", rs.getString(1)); assertEquals("\\", rs.getString(2)); assertEquals("don't", rs.getString(3)); } From 09ba73a9a5d37e408655d4525d0ec84d94227285 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Wed, 16 Oct 2024 11:18:00 +0100 Subject: [PATCH 03/13] fix newline and tests --- .../integration/tests/PreparedStatementTest.java | 16 +++++++++------- .../java/com/firebolt/jdbc/type/BaseType.java | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/integrationTest/java/integration/tests/PreparedStatementTest.java b/src/integrationTest/java/integration/tests/PreparedStatementTest.java index 36a81626f..84f44423d 100644 --- a/src/integrationTest/java/integration/tests/PreparedStatementTest.java +++ b/src/integrationTest/java/integration/tests/PreparedStatementTest.java @@ -439,16 +439,18 @@ void shouldInsertAndRetrieveUrl() throws SQLException, MalformedURLException { void shouldFetchSpecialCharacters() throws SQLException, MalformedURLException { try (Connection connection = createConnection()) { try (PreparedStatement statement = connection - .prepareStatement("SELECT ? as a, ? as b, ? as c ")) { - statement.setString(1, "\\0"); - statement.setString(2, "\\"); - statement.setString(3, "don't"); + .prepareStatement("SELECT ? as a, ? as b, ? as c, ? as d")) { + statement.setString(1, "ї"); + statement.setString(2, "\n"); + statement.setString(3, "\\"); + statement.setString(4, "don't"); statement.execute(); ResultSet rs = statement.getResultSet(); assertTrue(rs.next()); - assertEquals("\\0", rs.getString(1)); - assertEquals("\\", rs.getString(2)); - assertEquals("don't", rs.getString(3)); + assertEquals("ї", rs.getString(1)); + assertEquals("\n", rs.getString(2)); + assertEquals("\\", rs.getString(3)); + assertEquals("don't", rs.getString(4)); } } } diff --git a/src/main/java/com/firebolt/jdbc/type/BaseType.java b/src/main/java/com/firebolt/jdbc/type/BaseType.java index c2b4caf97..bf8677ddc 100644 --- a/src/main/java/com/firebolt/jdbc/type/BaseType.java +++ b/src/main/java/com/firebolt/jdbc/type/BaseType.java @@ -102,7 +102,7 @@ public enum BaseType { private static final class TypePredicate { private static final Predicate mayBeFloatingNumber = Pattern.compile("[.eE]").asPredicate(); } - public static final String NULL_VALUE = "\\N"; + public static final String NULL_VALUE = "NULL"; private final Class type; private final Predicate shouldTryFallback; private final CheckedFunction[] transformFunctions; From ea5721750a416860a48ee918476cab77df276d87 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Wed, 16 Oct 2024 11:43:06 +0100 Subject: [PATCH 04/13] better null check --- src/main/java/com/firebolt/jdbc/type/BaseType.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/firebolt/jdbc/type/BaseType.java b/src/main/java/com/firebolt/jdbc/type/BaseType.java index bf8677ddc..e99e0838f 100644 --- a/src/main/java/com/firebolt/jdbc/type/BaseType.java +++ b/src/main/java/com/firebolt/jdbc/type/BaseType.java @@ -102,7 +102,7 @@ public enum BaseType { private static final class TypePredicate { private static final Predicate mayBeFloatingNumber = Pattern.compile("[.eE]").asPredicate(); } - public static final String NULL_VALUE = "NULL"; + public static final String NULL_VALUE = "\\N"; private final Class type; private final Predicate shouldTryFallback; private final CheckedFunction[] transformFunctions; @@ -135,7 +135,7 @@ private static String checkInfinity(String s) { } public static boolean isNull(String value) { - return NULL_VALUE.equalsIgnoreCase(value); + return NULL_VALUE.equals(value); } private static boolean isNan(String value) { From 1d8e07af7af52d5b2b155be3b8224760c59dbcbc Mon Sep 17 00:00:00 2001 From: ptiurin Date: Wed, 16 Oct 2024 16:54:18 +0100 Subject: [PATCH 05/13] backwards-compatible change --- .../FireboltPreparedStatement.java | 8 +++++++- .../jdbc/type/JavaTypeToFireboltSQLString.java | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java index 41ce33462..083fe0ea4 100644 --- a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java +++ b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java @@ -2,6 +2,7 @@ import com.firebolt.jdbc.annotation.NotImplemented; import com.firebolt.jdbc.connection.FireboltConnection; +import com.firebolt.jdbc.connection.FireboltConnectionUserPassword; import com.firebolt.jdbc.connection.settings.FireboltProperties; import com.firebolt.jdbc.exception.ExceptionType; import com.firebolt.jdbc.exception.FireboltException; @@ -155,7 +156,12 @@ public void setBigDecimal(int parameterIndex, BigDecimal x) throws SQLException public void setString(int parameterIndex, String x) throws SQLException { validateStatementIsNotClosed(); validateParamIndex(parameterIndex); - providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.STRING.transform(x)); + if (this.getConnection().getClass() == FireboltConnectionUserPassword.class){ + // Old Firebolt required escaping additional characters in the string + providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.STRING.transform(x, "legacy")); + } else { + providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.STRING.transform(x)); + } } @Override diff --git a/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java b/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java index 7535d1b71..813354c87 100644 --- a/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java +++ b/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java @@ -35,7 +35,7 @@ public enum JavaTypeToFireboltSQLString { UUID(java.util.UUID.class, Object::toString), BYTE(Byte.class, value -> Byte.toString(((Number) value).byteValue())), SHORT(Short.class, value -> Short.toString(((Number) value).shortValue())), - STRING(String.class, getSQLStringValueOfString()), + STRING(String.class, getSQLStringValueOfString(), getSQLStringValueOfStringWithEscape()), LONG(Long.class, value -> Long.toString(((Number)value).longValue())), INTEGER(Integer.class, value -> Integer.toString(((Number)value).intValue())), BIG_INTEGER(BigInteger.class, value -> value instanceof BigInteger ? value.toString() : Long.toString(((Number)value).longValue())), @@ -47,6 +47,9 @@ public enum JavaTypeToFireboltSQLString { ARRAY(Array.class, SqlArrayUtil::arrayToString), BYTE_ARRAY(byte[].class, value -> ofNullable(byteArrayToHexString((byte[])value, true)).map(x -> format("E'%s'::BYTEA", x)).orElse(null)), ; + + private static final List> legacyCharacterToEscapedCharacterPairs = List.of( + Map.entry("\0", "\\0"), Map.entry("\\", "\\\\"), Map.entry("'", "''")); private static final List> characterToEscapedCharacterPairs = List.of( Map.entry("'", "''")); //https://docs.oracle.com/javase/1.5.0/docs/guide/jdbc/getstart/mapping.html @@ -133,9 +136,22 @@ private static Class getType(int sqlType) throws SQLException { } } + private static CheckedBiFunction getSQLStringValueOfStringWithEscape() { + return (value, escape) -> { + boolean legacyStringEscape = escape == "legacy"; + return getSQLStringValueOfString(legacyStringEscape).apply(value); + }; + } private static CheckedFunction getSQLStringValueOfString() { + return getSQLStringValueOfString(false); + } + + private static CheckedFunction getSQLStringValueOfString(boolean legacyStringEscape) { return value -> { String escaped = (String) value; + List> characterToEscapedCharacterPairs = legacyStringEscape + ? JavaTypeToFireboltSQLString.legacyCharacterToEscapedCharacterPairs + : JavaTypeToFireboltSQLString.characterToEscapedCharacterPairs; for (Entry specialCharacter : characterToEscapedCharacterPairs) { escaped = escaped.replace(specialCharacter.getKey(), specialCharacter.getValue()); } From fb94f40cd943bd861e6111ce0047f410fd65855b Mon Sep 17 00:00:00 2001 From: ptiurin Date: Fri, 18 Oct 2024 11:37:57 +0100 Subject: [PATCH 06/13] better version --- .../FireboltPreparedStatement.java | 12 +++++- .../firebolt/jdbc/type/FireboltVersion.java | 5 +++ .../type/JavaTypeToFireboltSQLString.java | 41 ++++++++++--------- 3 files changed, 38 insertions(+), 20 deletions(-) create mode 100644 src/main/java/com/firebolt/jdbc/type/FireboltVersion.java diff --git a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java index 083fe0ea4..62a4e4fad 100644 --- a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java +++ b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java @@ -13,6 +13,7 @@ import com.firebolt.jdbc.statement.StatementInfoWrapper; import com.firebolt.jdbc.statement.StatementUtil; import com.firebolt.jdbc.statement.rawstatement.RawStatementWrapper; +import com.firebolt.jdbc.type.FireboltVersion; import com.firebolt.jdbc.type.JavaTypeToFireboltSQLString; import com.firebolt.jdbc.util.InputStreamUtil; import lombok.CustomLog; @@ -158,7 +159,8 @@ public void setString(int parameterIndex, String x) throws SQLException { validateParamIndex(parameterIndex); if (this.getConnection().getClass() == FireboltConnectionUserPassword.class){ // Old Firebolt required escaping additional characters in the string - providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.STRING.transform(x, "legacy")); + providedParameters.put(parameterIndex, + JavaTypeToFireboltSQLString.STRING.transform(x, FireboltVersion.LEGACY)); } else { providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.STRING.transform(x)); } @@ -204,6 +206,14 @@ public void setObject(int parameterIndex, Object x, int targetSqlType) throws SQ validateStatementIsNotClosed(); validateParamIndex(parameterIndex); try { + if (this.getConnection().getClass() == FireboltConnectionUserPassword.class) { + // We don't know the targetSqlType, so we let JavaTypeToFireboltSQLString deal + // with legacy conversion + providedParameters.put(parameterIndex, + JavaTypeToFireboltSQLString.transformAny(x, targetSqlType, FireboltVersion.LEGACY)); + } else { + providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.transformAny(x, targetSqlType)); + } providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.transformAny(x, targetSqlType)); } catch (FireboltException fbe) { if (ExceptionType.TYPE_NOT_SUPPORTED.equals(fbe.getType())) { diff --git a/src/main/java/com/firebolt/jdbc/type/FireboltVersion.java b/src/main/java/com/firebolt/jdbc/type/FireboltVersion.java new file mode 100644 index 000000000..e48e1a06b --- /dev/null +++ b/src/main/java/com/firebolt/jdbc/type/FireboltVersion.java @@ -0,0 +1,5 @@ +package com.firebolt.jdbc.type; + +public enum FireboltVersion { + LEGACY, CURRENT +} diff --git a/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java b/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java index 813354c87..8057a92d7 100644 --- a/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java +++ b/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java @@ -35,7 +35,7 @@ public enum JavaTypeToFireboltSQLString { UUID(java.util.UUID.class, Object::toString), BYTE(Byte.class, value -> Byte.toString(((Number) value).byteValue())), SHORT(Short.class, value -> Short.toString(((Number) value).shortValue())), - STRING(String.class, getSQLStringValueOfString(), getSQLStringValueOfStringWithEscape()), + STRING(String.class, getSQLStringValueOfString(FireboltVersion.CURRENT), getSQLStringValueOfStringVersioned()), LONG(Long.class, value -> Long.toString(((Number)value).longValue())), INTEGER(Integer.class, value -> Integer.toString(((Number)value).intValue())), BIG_INTEGER(BigInteger.class, value -> value instanceof BigInteger ? value.toString() : Long.toString(((Number)value).longValue())), @@ -103,22 +103,33 @@ public enum JavaTypeToFireboltSQLString { } public static String transformAny(Object object) throws SQLException { - return transformAny(object, () -> getType(object)); + return transformAny(object, FireboltVersion.CURRENT); + } + + public static String transformAny(Object object, FireboltVersion version) throws SQLException { + return transformAny(object, () -> getType(object), version); } public static String transformAny(Object object, int sqlType) throws SQLException { - return transformAny(object, () -> getType(sqlType)); + return transformAny(object, sqlType, FireboltVersion.CURRENT); + } + + public static String transformAny(Object object, int sqlType, FireboltVersion version) throws SQLException { + return transformAny(object, () -> getType(sqlType), version); } - private static String transformAny(Object object, CheckedSupplier> classSupplier) throws SQLException { - return object == null ? NULL_VALUE : transformAny(object, classSupplier.get()); + private static String transformAny(Object object, CheckedSupplier> classSupplier, FireboltVersion version) throws SQLException { + return object == null ? NULL_VALUE : transformAny(object, classSupplier.get(), version); } - private static String transformAny(Object object, Class objectType) throws SQLException { + private static String transformAny(Object object, Class objectType, FireboltVersion version) throws SQLException { JavaTypeToFireboltSQLString converter = Optional.ofNullable(classToType.get(objectType)) .orElseThrow(() -> new FireboltException( format("Cannot convert type %s. The type is not supported.", objectType), TYPE_NOT_SUPPORTED)); + if (version == FireboltVersion.LEGACY && object instanceof String) { + return converter.transform(object, version); + } return converter.transform(object); } @@ -136,23 +147,15 @@ private static Class getType(int sqlType) throws SQLException { } } - private static CheckedBiFunction getSQLStringValueOfStringWithEscape() { - return (value, escape) -> { - boolean legacyStringEscape = escape == "legacy"; - return getSQLStringValueOfString(legacyStringEscape).apply(value); - }; - } - private static CheckedFunction getSQLStringValueOfString() { - return getSQLStringValueOfString(false); + private static CheckedBiFunction getSQLStringValueOfStringVersioned() { + return (value, version) -> getSQLStringValueOfString((FireboltVersion) version).apply(value); } - private static CheckedFunction getSQLStringValueOfString(boolean legacyStringEscape) { + private static CheckedFunction getSQLStringValueOfString(FireboltVersion version) { return value -> { String escaped = (String) value; - List> characterToEscapedCharacterPairs = legacyStringEscape - ? JavaTypeToFireboltSQLString.legacyCharacterToEscapedCharacterPairs - : JavaTypeToFireboltSQLString.characterToEscapedCharacterPairs; - for (Entry specialCharacter : characterToEscapedCharacterPairs) { + List> charactersToEscape = version == FireboltVersion.LEGACY ? legacyCharacterToEscapedCharacterPairs : characterToEscapedCharacterPairs; + for (Entry specialCharacter : charactersToEscape) { escaped = escaped.replace(specialCharacter.getKey(), specialCharacter.getValue()); } return format("'%s'", escaped); From a9e785aab50bf220d5d91eaabc3df2cb7fccf65b Mon Sep 17 00:00:00 2001 From: ptiurin Date: Fri, 18 Oct 2024 11:42:20 +0100 Subject: [PATCH 07/13] add tests --- .../type/JavaTypeToFireboltSQLStringTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java b/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java index 3a22fad33..0feab5860 100644 --- a/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java +++ b/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java @@ -80,6 +80,21 @@ void shouldEscapeCharactersWhenTransformingFromString() throws SQLException { assertEquals("'105\\'", JavaTypeToFireboltSQLString.transformAny("105\\")); } + @Test + void shouldEscapeCharactersWhenTransformingFromStringLegacy() throws SQLException { + // quotes are escaped + assertEquals("'105'' OR 1=1--'' '", + JavaTypeToFireboltSQLString.STRING.transform("105' OR 1=1--' ", FireboltVersion.LEGACY)); + assertEquals("'105'' OR 1=1--'' '", + JavaTypeToFireboltSQLString.transformAny("105' OR 1=1--' ", FireboltVersion.LEGACY)); + // \0 is escaped + assertEquals("'105\\\\0'", JavaTypeToFireboltSQLString.STRING.transform("105\0", FireboltVersion.LEGACY)); + assertEquals("'105\\\\0'", JavaTypeToFireboltSQLString.transformAny("105\0", FireboltVersion.LEGACY)); + // backslashes are escaped + assertEquals("'105\\\\'", JavaTypeToFireboltSQLString.STRING.transform("105\\", FireboltVersion.LEGACY)); + assertEquals("'105\\\\'", JavaTypeToFireboltSQLString.transformAny("105\\", FireboltVersion.LEGACY)); + } + @Test void shouldTransformLongToString() throws SQLException { assertEquals("105", JavaTypeToFireboltSQLString.LONG.transform(105L)); From be088703ae5561ebf0e39536b37b11fda472517a Mon Sep 17 00:00:00 2001 From: ptiurin Date: Fri, 18 Oct 2024 11:47:27 +0100 Subject: [PATCH 08/13] rename --- .../FireboltPreparedStatement.java | 6 ++--- .../type/JavaTypeToFireboltSQLString.java | 22 +++++++++---------- ...ireboltVersion.java => ParserVersion.java} | 2 +- .../type/JavaTypeToFireboltSQLStringTest.java | 12 +++++----- 4 files changed, 21 insertions(+), 21 deletions(-) rename src/main/java/com/firebolt/jdbc/type/{FireboltVersion.java => ParserVersion.java} (64%) diff --git a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java index 62a4e4fad..61293c15b 100644 --- a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java +++ b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java @@ -13,7 +13,7 @@ import com.firebolt.jdbc.statement.StatementInfoWrapper; import com.firebolt.jdbc.statement.StatementUtil; import com.firebolt.jdbc.statement.rawstatement.RawStatementWrapper; -import com.firebolt.jdbc.type.FireboltVersion; +import com.firebolt.jdbc.type.ParserVersion; import com.firebolt.jdbc.type.JavaTypeToFireboltSQLString; import com.firebolt.jdbc.util.InputStreamUtil; import lombok.CustomLog; @@ -160,7 +160,7 @@ public void setString(int parameterIndex, String x) throws SQLException { if (this.getConnection().getClass() == FireboltConnectionUserPassword.class){ // Old Firebolt required escaping additional characters in the string providedParameters.put(parameterIndex, - JavaTypeToFireboltSQLString.STRING.transform(x, FireboltVersion.LEGACY)); + JavaTypeToFireboltSQLString.STRING.transform(x, ParserVersion.LEGACY)); } else { providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.STRING.transform(x)); } @@ -210,7 +210,7 @@ public void setObject(int parameterIndex, Object x, int targetSqlType) throws SQ // We don't know the targetSqlType, so we let JavaTypeToFireboltSQLString deal // with legacy conversion providedParameters.put(parameterIndex, - JavaTypeToFireboltSQLString.transformAny(x, targetSqlType, FireboltVersion.LEGACY)); + JavaTypeToFireboltSQLString.transformAny(x, targetSqlType, ParserVersion.LEGACY)); } else { providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.transformAny(x, targetSqlType)); } diff --git a/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java b/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java index 8057a92d7..cb3db7ba9 100644 --- a/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java +++ b/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java @@ -35,7 +35,7 @@ public enum JavaTypeToFireboltSQLString { UUID(java.util.UUID.class, Object::toString), BYTE(Byte.class, value -> Byte.toString(((Number) value).byteValue())), SHORT(Short.class, value -> Short.toString(((Number) value).shortValue())), - STRING(String.class, getSQLStringValueOfString(FireboltVersion.CURRENT), getSQLStringValueOfStringVersioned()), + STRING(String.class, getSQLStringValueOfString(ParserVersion.CURRENT), getSQLStringValueOfStringVersioned()), LONG(Long.class, value -> Long.toString(((Number)value).longValue())), INTEGER(Integer.class, value -> Integer.toString(((Number)value).intValue())), BIG_INTEGER(BigInteger.class, value -> value instanceof BigInteger ? value.toString() : Long.toString(((Number)value).longValue())), @@ -103,31 +103,31 @@ public enum JavaTypeToFireboltSQLString { } public static String transformAny(Object object) throws SQLException { - return transformAny(object, FireboltVersion.CURRENT); + return transformAny(object, ParserVersion.CURRENT); } - public static String transformAny(Object object, FireboltVersion version) throws SQLException { + public static String transformAny(Object object, ParserVersion version) throws SQLException { return transformAny(object, () -> getType(object), version); } public static String transformAny(Object object, int sqlType) throws SQLException { - return transformAny(object, sqlType, FireboltVersion.CURRENT); + return transformAny(object, sqlType, ParserVersion.CURRENT); } - public static String transformAny(Object object, int sqlType, FireboltVersion version) throws SQLException { + public static String transformAny(Object object, int sqlType, ParserVersion version) throws SQLException { return transformAny(object, () -> getType(sqlType), version); } - private static String transformAny(Object object, CheckedSupplier> classSupplier, FireboltVersion version) throws SQLException { + private static String transformAny(Object object, CheckedSupplier> classSupplier, ParserVersion version) throws SQLException { return object == null ? NULL_VALUE : transformAny(object, classSupplier.get(), version); } - private static String transformAny(Object object, Class objectType, FireboltVersion version) throws SQLException { + private static String transformAny(Object object, Class objectType, ParserVersion version) throws SQLException { JavaTypeToFireboltSQLString converter = Optional.ofNullable(classToType.get(objectType)) .orElseThrow(() -> new FireboltException( format("Cannot convert type %s. The type is not supported.", objectType), TYPE_NOT_SUPPORTED)); - if (version == FireboltVersion.LEGACY && object instanceof String) { + if (version == ParserVersion.LEGACY && object instanceof String) { return converter.transform(object, version); } return converter.transform(object); @@ -148,13 +148,13 @@ private static Class getType(int sqlType) throws SQLException { } private static CheckedBiFunction getSQLStringValueOfStringVersioned() { - return (value, version) -> getSQLStringValueOfString((FireboltVersion) version).apply(value); + return (value, version) -> getSQLStringValueOfString((ParserVersion) version).apply(value); } - private static CheckedFunction getSQLStringValueOfString(FireboltVersion version) { + private static CheckedFunction getSQLStringValueOfString(ParserVersion version) { return value -> { String escaped = (String) value; - List> charactersToEscape = version == FireboltVersion.LEGACY ? legacyCharacterToEscapedCharacterPairs : characterToEscapedCharacterPairs; + List> charactersToEscape = version == ParserVersion.LEGACY ? legacyCharacterToEscapedCharacterPairs : characterToEscapedCharacterPairs; for (Entry specialCharacter : charactersToEscape) { escaped = escaped.replace(specialCharacter.getKey(), specialCharacter.getValue()); } diff --git a/src/main/java/com/firebolt/jdbc/type/FireboltVersion.java b/src/main/java/com/firebolt/jdbc/type/ParserVersion.java similarity index 64% rename from src/main/java/com/firebolt/jdbc/type/FireboltVersion.java rename to src/main/java/com/firebolt/jdbc/type/ParserVersion.java index e48e1a06b..31a061212 100644 --- a/src/main/java/com/firebolt/jdbc/type/FireboltVersion.java +++ b/src/main/java/com/firebolt/jdbc/type/ParserVersion.java @@ -1,5 +1,5 @@ package com.firebolt.jdbc.type; -public enum FireboltVersion { +public enum ParserVersion { LEGACY, CURRENT } diff --git a/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java b/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java index 0feab5860..7e7b24ff1 100644 --- a/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java +++ b/src/test/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLStringTest.java @@ -84,15 +84,15 @@ void shouldEscapeCharactersWhenTransformingFromString() throws SQLException { void shouldEscapeCharactersWhenTransformingFromStringLegacy() throws SQLException { // quotes are escaped assertEquals("'105'' OR 1=1--'' '", - JavaTypeToFireboltSQLString.STRING.transform("105' OR 1=1--' ", FireboltVersion.LEGACY)); + JavaTypeToFireboltSQLString.STRING.transform("105' OR 1=1--' ", ParserVersion.LEGACY)); assertEquals("'105'' OR 1=1--'' '", - JavaTypeToFireboltSQLString.transformAny("105' OR 1=1--' ", FireboltVersion.LEGACY)); + JavaTypeToFireboltSQLString.transformAny("105' OR 1=1--' ", ParserVersion.LEGACY)); // \0 is escaped - assertEquals("'105\\\\0'", JavaTypeToFireboltSQLString.STRING.transform("105\0", FireboltVersion.LEGACY)); - assertEquals("'105\\\\0'", JavaTypeToFireboltSQLString.transformAny("105\0", FireboltVersion.LEGACY)); + assertEquals("'105\\\\0'", JavaTypeToFireboltSQLString.STRING.transform("105\0", ParserVersion.LEGACY)); + assertEquals("'105\\\\0'", JavaTypeToFireboltSQLString.transformAny("105\0", ParserVersion.LEGACY)); // backslashes are escaped - assertEquals("'105\\\\'", JavaTypeToFireboltSQLString.STRING.transform("105\\", FireboltVersion.LEGACY)); - assertEquals("'105\\\\'", JavaTypeToFireboltSQLString.transformAny("105\\", FireboltVersion.LEGACY)); + assertEquals("'105\\\\'", JavaTypeToFireboltSQLString.STRING.transform("105\\", ParserVersion.LEGACY)); + assertEquals("'105\\\\'", JavaTypeToFireboltSQLString.transformAny("105\\", ParserVersion.LEGACY)); } @Test From 9a7d56b58663926b7afaa596c54072c8f85f9887 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 21 Oct 2024 09:49:03 +0100 Subject: [PATCH 09/13] fix setObject --- .../statement/preparedstatement/FireboltPreparedStatement.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java index 61293c15b..45c23e6cc 100644 --- a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java +++ b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java @@ -214,7 +214,6 @@ public void setObject(int parameterIndex, Object x, int targetSqlType) throws SQ } else { providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.transformAny(x, targetSqlType)); } - providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.transformAny(x, targetSqlType)); } catch (FireboltException fbe) { if (ExceptionType.TYPE_NOT_SUPPORTED.equals(fbe.getType())) { throw new SQLFeatureNotSupportedException(fbe.getMessage(), fbe); From 047a6dc6ebbd04663919e850ee0a3d9645afa63c Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 21 Oct 2024 10:36:16 +0100 Subject: [PATCH 10/13] Refactor into ConnectionParserResolver --- .../FireboltPreparedStatement.java | 22 +++++----------- .../jdbc/util/ConnectionParserResolver.java | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 16 deletions(-) create mode 100644 src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java diff --git a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java index 45c23e6cc..86f4775bd 100644 --- a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java +++ b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java @@ -2,7 +2,6 @@ import com.firebolt.jdbc.annotation.NotImplemented; import com.firebolt.jdbc.connection.FireboltConnection; -import com.firebolt.jdbc.connection.FireboltConnectionUserPassword; import com.firebolt.jdbc.connection.settings.FireboltProperties; import com.firebolt.jdbc.exception.ExceptionType; import com.firebolt.jdbc.exception.FireboltException; @@ -15,6 +14,7 @@ import com.firebolt.jdbc.statement.rawstatement.RawStatementWrapper; import com.firebolt.jdbc.type.ParserVersion; import com.firebolt.jdbc.type.JavaTypeToFireboltSQLString; +import com.firebolt.jdbc.util.ConnectionParserResolver; import com.firebolt.jdbc.util.InputStreamUtil; import lombok.CustomLog; import lombok.NonNull; @@ -59,6 +59,7 @@ public class FireboltPreparedStatement extends FireboltStatement implements Prep private final RawStatementWrapper rawStatement; private final List> rows; private Map providedParameters; + private final ParserVersion parserVersion; public FireboltPreparedStatement(FireboltStatementService statementService, FireboltConnection connection, String sql) { this(statementService, connection.getSessionProperties(), connection, sql); @@ -72,6 +73,7 @@ public FireboltPreparedStatement(FireboltStatementService statementService, Fire this.rawStatement = StatementUtil.parseToRawStatementWrapper(sql); rawStatement.getSubStatements().forEach(statement -> createValidator(statement, connection).validate(statement)); this.rows = new ArrayList<>(); + this.parserVersion = ConnectionParserResolver.getParserFromConnection(connection); } @Override @@ -157,13 +159,7 @@ public void setBigDecimal(int parameterIndex, BigDecimal x) throws SQLException public void setString(int parameterIndex, String x) throws SQLException { validateStatementIsNotClosed(); validateParamIndex(parameterIndex); - if (this.getConnection().getClass() == FireboltConnectionUserPassword.class){ - // Old Firebolt required escaping additional characters in the string - providedParameters.put(parameterIndex, - JavaTypeToFireboltSQLString.STRING.transform(x, ParserVersion.LEGACY)); - } else { - providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.STRING.transform(x)); - } + providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.STRING.transform(x, parserVersion)); } @Override @@ -206,14 +202,8 @@ public void setObject(int parameterIndex, Object x, int targetSqlType) throws SQ validateStatementIsNotClosed(); validateParamIndex(parameterIndex); try { - if (this.getConnection().getClass() == FireboltConnectionUserPassword.class) { - // We don't know the targetSqlType, so we let JavaTypeToFireboltSQLString deal - // with legacy conversion - providedParameters.put(parameterIndex, - JavaTypeToFireboltSQLString.transformAny(x, targetSqlType, ParserVersion.LEGACY)); - } else { - providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.transformAny(x, targetSqlType)); - } + providedParameters.put(parameterIndex, + JavaTypeToFireboltSQLString.transformAny(x, targetSqlType, parserVersion)); } catch (FireboltException fbe) { if (ExceptionType.TYPE_NOT_SUPPORTED.equals(fbe.getType())) { throw new SQLFeatureNotSupportedException(fbe.getMessage(), fbe); diff --git a/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java b/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java new file mode 100644 index 000000000..03e32f381 --- /dev/null +++ b/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java @@ -0,0 +1,25 @@ +package com.firebolt.jdbc.util; + +import com.firebolt.jdbc.type.ParserVersion; +import lombok.CustomLog; + +import java.sql.Connection; + +@CustomLog +public class ConnectionParserResolver { + private ConnectionParserResolver() { + // Private constructor to prevent instantiation + } + + public static ParserVersion getParserFromConnection(Connection connection) { + if (connection instanceof com.firebolt.jdbc.connection.FireboltConnectionUserPassword) { + return ParserVersion.LEGACY; + } else if (connection instanceof com.firebolt.jdbc.connection.FireboltConnectionServiceSecret) { + return ParserVersion.CURRENT; + } else { + log.warn("Connection instance is not recognized, assuming \"current\" sql string parser: " + + connection.getClass().getName()); + return ParserVersion.CURRENT; + } + } +} From 78d6f305220146e6a8e8b208bb8aa8cb7d622d16 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 21 Oct 2024 10:56:49 +0100 Subject: [PATCH 11/13] Cleaner version --- .../jdbc/util/ConnectionParserResolver.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java b/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java index 03e32f381..b53d1dcbc 100644 --- a/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java +++ b/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java @@ -4,22 +4,22 @@ import lombok.CustomLog; import java.sql.Connection; +import java.util.HashMap; +import java.util.Map; @CustomLog public class ConnectionParserResolver { + // Map of classes to parser versions + private static final Map, ParserVersion> PARSER_MAP = new HashMap<>( + Map.of( + com.firebolt.jdbc.connection.FireboltConnectionUserPassword.class, ParserVersion.LEGACY, + com.firebolt.jdbc.connection.FireboltConnectionServiceSecret.class, ParserVersion.CURRENT)); + private ConnectionParserResolver() { // Private constructor to prevent instantiation } public static ParserVersion getParserFromConnection(Connection connection) { - if (connection instanceof com.firebolt.jdbc.connection.FireboltConnectionUserPassword) { - return ParserVersion.LEGACY; - } else if (connection instanceof com.firebolt.jdbc.connection.FireboltConnectionServiceSecret) { - return ParserVersion.CURRENT; - } else { - log.warn("Connection instance is not recognized, assuming \"current\" sql string parser: " - + connection.getClass().getName()); - return ParserVersion.CURRENT; - } + return PARSER_MAP.getOrDefault(connection.getClass(), ParserVersion.CURRENT); } } From 8fad16f36ad6fbd312e7e150fac55cd020cd1b1e Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 21 Oct 2024 11:21:14 +0100 Subject: [PATCH 12/13] Better --- .../jdbc/connection/FireboltConnection.java | 19 +++++++++++--- .../FireboltConnectionServiceSecret.java | 12 ++++++--- .../FireboltConnectionUserPassword.java | 12 ++++++--- .../FireboltPreparedStatement.java | 3 +-- .../jdbc/util/ConnectionParserResolver.java | 25 ------------------- .../FireboltConnectionServiceSecretTest.java | 8 ++++-- .../FireboltConnectionUserPasswordTest.java | 4 ++- 7 files changed, 41 insertions(+), 42 deletions(-) delete mode 100644 src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java diff --git a/src/main/java/com/firebolt/jdbc/connection/FireboltConnection.java b/src/main/java/com/firebolt/jdbc/connection/FireboltConnection.java index 1129935f8..1b5976229 100644 --- a/src/main/java/com/firebolt/jdbc/connection/FireboltConnection.java +++ b/src/main/java/com/firebolt/jdbc/connection/FireboltConnection.java @@ -18,11 +18,13 @@ import com.firebolt.jdbc.statement.FireboltStatement; import com.firebolt.jdbc.statement.preparedstatement.FireboltPreparedStatement; import com.firebolt.jdbc.type.FireboltDataType; +import com.firebolt.jdbc.type.ParserVersion; import com.firebolt.jdbc.type.array.FireboltArray; import com.firebolt.jdbc.type.lob.FireboltBlob; import com.firebolt.jdbc.type.lob.FireboltClob; import com.firebolt.jdbc.util.PropertyUtil; import lombok.CustomLog; +import lombok.Getter; import lombok.NonNull; import okhttp3.OkHttpClient; @@ -83,12 +85,16 @@ public abstract class FireboltConnection extends JdbcBase implements Connection, //Properties that are used at the beginning of the connection for authentication protected final FireboltProperties loginProperties; private final Collection cacheListeners = Collections.newSetFromMap(new IdentityHashMap<>()); + // Parameter parser is determined by the version we're running on + @Getter + public final ParserVersion parserVersion; protected FireboltConnection(@NonNull String url, Properties connectionSettings, FireboltAuthenticationService fireboltAuthenticationService, FireboltStatementService fireboltStatementService, - String protocolVersion) { + String protocolVersion, + ParserVersion parserVersion) { this.loginProperties = extractFireboltProperties(url, connectionSettings); this.fireboltAuthenticationService = fireboltAuthenticationService; @@ -99,11 +105,13 @@ protected FireboltConnection(@NonNull String url, this.connectionTimeout = loginProperties.getConnectionTimeoutMillis(); this.networkTimeout = loginProperties.getSocketTimeoutMillis(); this.protocolVersion = protocolVersion; + this.parserVersion = parserVersion; } // This code duplication between constructors is done because of back reference: dependent services require reference to current instance of FireboltConnection that prevents using constructor chaining or factory method. @ExcludeFromJacocoGeneratedReport - protected FireboltConnection(@NonNull String url, Properties connectionSettings, String protocolVersion) throws SQLException { + protected FireboltConnection(@NonNull String url, Properties connectionSettings, String protocolVersion, + ParserVersion parserVersion) throws SQLException { this.loginProperties = extractFireboltProperties(url, connectionSettings); OkHttpClient httpClient = getHttpClient(loginProperties); @@ -115,6 +123,7 @@ protected FireboltConnection(@NonNull String url, Properties connectionSettings, this.connectionTimeout = loginProperties.getConnectionTimeoutMillis(); this.networkTimeout = loginProperties.getSocketTimeoutMillis(); this.protocolVersion = protocolVersion; + this.parserVersion = parserVersion; } protected abstract FireboltAuthenticationClient createFireboltAuthenticationClient(OkHttpClient httpClient); @@ -125,8 +134,10 @@ public static FireboltConnection create(@NonNull String url, Properties connecti private static FireboltConnection createConnectionInstance(@NonNull String url, Properties connectionSettings) throws SQLException { switch(getUrlVersion(url, connectionSettings)) { - case 1: return new FireboltConnectionUserPassword(url, connectionSettings); - case 2: return new FireboltConnectionServiceSecret(url, connectionSettings); + case 1: + return new FireboltConnectionUserPassword(url, connectionSettings, ParserVersion.LEGACY); + case 2: + return new FireboltConnectionServiceSecret(url, connectionSettings, ParserVersion.CURRENT); default: throw new IllegalArgumentException(format("Cannot distinguish version from url %s", url)); } } diff --git a/src/main/java/com/firebolt/jdbc/connection/FireboltConnectionServiceSecret.java b/src/main/java/com/firebolt/jdbc/connection/FireboltConnectionServiceSecret.java index 72122f9a4..9fb793e35 100644 --- a/src/main/java/com/firebolt/jdbc/connection/FireboltConnectionServiceSecret.java +++ b/src/main/java/com/firebolt/jdbc/connection/FireboltConnectionServiceSecret.java @@ -16,6 +16,7 @@ import com.firebolt.jdbc.service.FireboltEngineVersion2Service; import com.firebolt.jdbc.service.FireboltGatewayUrlService; import com.firebolt.jdbc.service.FireboltStatementService; +import com.firebolt.jdbc.type.ParserVersion; import com.firebolt.jdbc.util.PropertyUtil; import lombok.NonNull; import okhttp3.OkHttpClient; @@ -42,16 +43,19 @@ public class FireboltConnectionServiceSecret extends FireboltConnection { FireboltGatewayUrlService fireboltGatewayUrlService, FireboltStatementService fireboltStatementService, FireboltEngineInformationSchemaService fireboltEngineService, - FireboltAccountIdService fireboltAccountIdService) throws SQLException { - super(url, connectionSettings, fireboltAuthenticationService, fireboltStatementService, PROTOCOL_VERSION); + FireboltAccountIdService fireboltAccountIdService, + ParserVersion parserVersion) throws SQLException { + super(url, connectionSettings, fireboltAuthenticationService, fireboltStatementService, PROTOCOL_VERSION, + parserVersion); this.fireboltGatewayUrlService = fireboltGatewayUrlService; this.fireboltEngineService = fireboltEngineService; connect(); } @ExcludeFromJacocoGeneratedReport - FireboltConnectionServiceSecret(@NonNull String url, Properties connectionSettings) throws SQLException { - super(url, connectionSettings, PROTOCOL_VERSION); + FireboltConnectionServiceSecret(@NonNull String url, Properties connectionSettings, ParserVersion parserVersion) + throws SQLException { + super(url, connectionSettings, PROTOCOL_VERSION, parserVersion); OkHttpClient httpClient = getHttpClient(loginProperties); this.fireboltGatewayUrlService = new FireboltGatewayUrlService(createFireboltAccountRetriever(httpClient, GatewayUrlResponse.class)); // initialization of fireboltEngineService depends on the infraVersion (the version of engine) diff --git a/src/main/java/com/firebolt/jdbc/connection/FireboltConnectionUserPassword.java b/src/main/java/com/firebolt/jdbc/connection/FireboltConnectionUserPassword.java index e36845c1b..84058571b 100644 --- a/src/main/java/com/firebolt/jdbc/connection/FireboltConnectionUserPassword.java +++ b/src/main/java/com/firebolt/jdbc/connection/FireboltConnectionUserPassword.java @@ -11,6 +11,7 @@ import com.firebolt.jdbc.service.FireboltEngineInformationSchemaService; import com.firebolt.jdbc.service.FireboltEngineService; import com.firebolt.jdbc.service.FireboltStatementService; +import com.firebolt.jdbc.type.ParserVersion; import lombok.NonNull; import okhttp3.OkHttpClient; @@ -27,15 +28,18 @@ public class FireboltConnectionUserPassword extends FireboltConnection { Properties connectionSettings, FireboltAuthenticationService fireboltAuthenticationService, FireboltStatementService fireboltStatementService, - FireboltEngineInformationSchemaService fireboltEngineService) throws SQLException { - super(url, connectionSettings, fireboltAuthenticationService, fireboltStatementService, PROTOCOL_VERSION); + FireboltEngineInformationSchemaService fireboltEngineService, + ParserVersion parserVersion) throws SQLException { + super(url, connectionSettings, fireboltAuthenticationService, fireboltStatementService, PROTOCOL_VERSION, + parserVersion); this.fireboltEngineService = fireboltEngineService; connect(); } @ExcludeFromJacocoGeneratedReport - FireboltConnectionUserPassword(@NonNull String url, Properties connectionSettings) throws SQLException { - super(url, connectionSettings, PROTOCOL_VERSION); + FireboltConnectionUserPassword(@NonNull String url, Properties connectionSettings, ParserVersion parserVersion) + throws SQLException { + super(url, connectionSettings, PROTOCOL_VERSION, parserVersion); OkHttpClient httpClient = getHttpClient(loginProperties); this.fireboltEngineService = new FireboltEngineApiService(new FireboltAccountClient(httpClient, this, loginProperties.getUserDrivers(), loginProperties.getUserClients())); connect(); diff --git a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java index 86f4775bd..12e479b04 100644 --- a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java +++ b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java @@ -14,7 +14,6 @@ import com.firebolt.jdbc.statement.rawstatement.RawStatementWrapper; import com.firebolt.jdbc.type.ParserVersion; import com.firebolt.jdbc.type.JavaTypeToFireboltSQLString; -import com.firebolt.jdbc.util.ConnectionParserResolver; import com.firebolt.jdbc.util.InputStreamUtil; import lombok.CustomLog; import lombok.NonNull; @@ -73,7 +72,7 @@ public FireboltPreparedStatement(FireboltStatementService statementService, Fire this.rawStatement = StatementUtil.parseToRawStatementWrapper(sql); rawStatement.getSubStatements().forEach(statement -> createValidator(statement, connection).validate(statement)); this.rows = new ArrayList<>(); - this.parserVersion = ConnectionParserResolver.getParserFromConnection(connection); + this.parserVersion = connection.getParserVersion(); } @Override diff --git a/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java b/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java deleted file mode 100644 index b53d1dcbc..000000000 --- a/src/main/java/com/firebolt/jdbc/util/ConnectionParserResolver.java +++ /dev/null @@ -1,25 +0,0 @@ -package com.firebolt.jdbc.util; - -import com.firebolt.jdbc.type.ParserVersion; -import lombok.CustomLog; - -import java.sql.Connection; -import java.util.HashMap; -import java.util.Map; - -@CustomLog -public class ConnectionParserResolver { - // Map of classes to parser versions - private static final Map, ParserVersion> PARSER_MAP = new HashMap<>( - Map.of( - com.firebolt.jdbc.connection.FireboltConnectionUserPassword.class, ParserVersion.LEGACY, - com.firebolt.jdbc.connection.FireboltConnectionServiceSecret.class, ParserVersion.CURRENT)); - - private ConnectionParserResolver() { - // Private constructor to prevent instantiation - } - - public static ParserVersion getParserFromConnection(Connection connection) { - return PARSER_MAP.getOrDefault(connection.getClass(), ParserVersion.CURRENT); - } -} diff --git a/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionServiceSecretTest.java b/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionServiceSecretTest.java index 4c02fc295..381d01f05 100644 --- a/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionServiceSecretTest.java +++ b/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionServiceSecretTest.java @@ -5,6 +5,7 @@ import com.firebolt.jdbc.connection.settings.FireboltProperties; import com.firebolt.jdbc.exception.FireboltException; import com.firebolt.jdbc.service.FireboltGatewayUrlService; +import com.firebolt.jdbc.type.ParserVersion; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; @@ -100,7 +101,9 @@ void checkSystemEngineEndpoint(String gatewayUrl, String expectedHost, String ex @SuppressWarnings("unchecked") FireboltAccountRetriever fireboltGatewayUrlClient = mock(FireboltAccountRetriever.class); when(fireboltGatewayUrlClient.retrieve(any(), any())).thenReturn(new GatewayUrlResponse(gatewayUrl)); FireboltGatewayUrlService gatewayUrlService = new FireboltGatewayUrlService(fireboltGatewayUrlClient); - FireboltConnection connection = new FireboltConnectionServiceSecret(SYSTEM_ENGINE_URL, connectionProperties, fireboltAuthenticationService, gatewayUrlService, fireboltStatementService, fireboltEngineService, fireboltAccountIdService); + FireboltConnection connection = new FireboltConnectionServiceSecret(SYSTEM_ENGINE_URL, connectionProperties, + fireboltAuthenticationService, gatewayUrlService, fireboltStatementService, fireboltEngineService, + fireboltAccountIdService, ParserVersion.CURRENT); FireboltProperties sessionProperties = connection.getSessionProperties(); assertEquals(expectedHost, sessionProperties.getHost()); assertEquals(expectedProps == null ? Map.of() : Arrays.stream(expectedProps.split(";")).map(kv -> kv.split("=")).collect(toMap(kv -> kv[0], kv -> kv[1])), sessionProperties.getAdditionalProperties()); @@ -113,6 +116,7 @@ void shouldNotFetchTokenNorEngineHostForLocalFirebolt() throws SQLException { } protected FireboltConnection createConnection(String url, Properties props) throws SQLException { - return new FireboltConnectionServiceSecret(url, props, fireboltAuthenticationService, fireboltGatewayUrlService, fireboltStatementService, fireboltEngineService, fireboltAccountIdService); + return new FireboltConnectionServiceSecret(url, props, fireboltAuthenticationService, fireboltGatewayUrlService, + fireboltStatementService, fireboltEngineService, fireboltAccountIdService, ParserVersion.CURRENT); } } diff --git a/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionUserPasswordTest.java b/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionUserPasswordTest.java index 927fcdf34..473f72bc3 100644 --- a/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionUserPasswordTest.java +++ b/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionUserPasswordTest.java @@ -1,5 +1,6 @@ package com.firebolt.jdbc.connection; +import com.firebolt.jdbc.type.ParserVersion; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; @@ -73,6 +74,7 @@ void getMetadata(String engine) throws SQLException { } protected FireboltConnection createConnection(String url, Properties props) throws SQLException { - return new FireboltConnectionUserPassword(url, props, fireboltAuthenticationService, fireboltStatementService, fireboltEngineService); + return new FireboltConnectionUserPassword(url, props, fireboltAuthenticationService, fireboltStatementService, + fireboltEngineService, ParserVersion.LEGACY); } } From 676748ab008d37d8421ce4fc77b07c9f484c0f32 Mon Sep 17 00:00:00 2001 From: ptiurin Date: Mon, 21 Oct 2024 11:25:41 +0100 Subject: [PATCH 13/13] missing constructor --- .../firebolt/jdbc/client/query/StatementClientImplTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/firebolt/jdbc/client/query/StatementClientImplTest.java b/src/test/java/com/firebolt/jdbc/client/query/StatementClientImplTest.java index eae589a79..e6c0e5b0a 100644 --- a/src/test/java/com/firebolt/jdbc/client/query/StatementClientImplTest.java +++ b/src/test/java/com/firebolt/jdbc/client/query/StatementClientImplTest.java @@ -10,6 +10,7 @@ import com.firebolt.jdbc.exception.FireboltException; import com.firebolt.jdbc.statement.StatementInfoWrapper; import com.firebolt.jdbc.statement.StatementUtil; +import com.firebolt.jdbc.type.ParserVersion; import lombok.NonNull; import okhttp3.Call; import okhttp3.Dispatcher; @@ -349,7 +350,7 @@ private FireboltConnection use(int mockedInfraVersion, Properties props, String Call useCall = getMockedCallWithResponse(200, "", responseHeaders); Call select1Call = getMockedCallWithResponse(200, ""); when(okHttpClient.newCall(any())).thenReturn(useCall, select1Call); - FireboltConnection connection = new FireboltConnection("url", props, "0") { + FireboltConnection connection = new FireboltConnection("url", props, "0", ParserVersion.CURRENT) { { this.infraVersion = mockedInfraVersion; try {