Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(FIR-37280): parameter formatting for the new Firebolt #464

Merged
merged 14 commits into from
Oct 21, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,26 @@ 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, ? 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("ї", rs.getString(1));
assertEquals("\n", rs.getString(2));
assertEquals("\\", rs.getString(3));
assertEquals("don't", rs.getString(4));
}
}
}

@Builder
@Value
@EqualsAndHashCode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -12,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.ParserVersion;
import com.firebolt.jdbc.type.JavaTypeToFireboltSQLString;
import com.firebolt.jdbc.util.InputStreamUtil;
import lombok.CustomLog;
Expand Down Expand Up @@ -155,7 +157,13 @@ 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, ParserVersion.LEGACY));
} else {
providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.STRING.transform(x));
}
}

@Override
Expand Down Expand Up @@ -198,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, ParserVersion.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())) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/firebolt/jdbc/type/BaseType.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(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())),
Expand All @@ -47,8 +47,11 @@ 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<Entry<String, String>> characterToEscapedCharacterPairs = List.of(

private static final List<Entry<String, String>> legacyCharacterToEscapedCharacterPairs = List.of(
Map.entry("\0", "\\0"), Map.entry("\\", "\\\\"), Map.entry("'", "''"));
private static final List<Entry<String, String>> characterToEscapedCharacterPairs = List.of(
Map.entry("'", "''"));
//https://docs.oracle.com/javase/1.5.0/docs/guide/jdbc/getstart/mapping.html
private static final Map<JDBCType, Class<?>> jdbcTypeToClass = Map.ofEntries(
Map.entry(JDBCType.CHAR, String.class),
Expand Down Expand Up @@ -100,22 +103,33 @@ public enum JavaTypeToFireboltSQLString {
}

public static String transformAny(Object object) throws SQLException {
return transformAny(object, () -> getType(object));
return transformAny(object, ParserVersion.CURRENT);
}

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, () -> getType(sqlType));
return transformAny(object, sqlType, ParserVersion.CURRENT);
}

private static String transformAny(Object object, CheckedSupplier<Class<?>> classSupplier) throws SQLException {
return object == null ? NULL_VALUE : transformAny(object, classSupplier.get());
public static String transformAny(Object object, int sqlType, ParserVersion version) throws SQLException {
return transformAny(object, () -> getType(sqlType), version);
}

private static String transformAny(Object object, Class<?> objectType) throws SQLException {
private static String transformAny(Object object, CheckedSupplier<Class<?>> classSupplier, ParserVersion version) throws SQLException {
return object == null ? NULL_VALUE : transformAny(object, classSupplier.get(), version);
}

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 == ParserVersion.LEGACY && object instanceof String) {
return converter.transform(object, version);
}
return converter.transform(object);
}

Expand All @@ -133,10 +147,15 @@ private static Class<?> getType(int sqlType) throws SQLException {
}
}

private static CheckedFunction<Object, String> getSQLStringValueOfString() {
private static CheckedBiFunction<Object, Object, String> getSQLStringValueOfStringVersioned() {
return (value, version) -> getSQLStringValueOfString((ParserVersion) version).apply(value);
}

private static CheckedFunction<Object, String> getSQLStringValueOfString(ParserVersion version) {
return value -> {
String escaped = (String) value;
for (Entry<String, String> specialCharacter : characterToEscapedCharacterPairs) {
List<Entry<String, String>> charactersToEscape = version == ParserVersion.LEGACY ? legacyCharacterToEscapedCharacterPairs : characterToEscapedCharacterPairs;
for (Entry<String, String> specialCharacter : charactersToEscape) {
escaped = escaped.replace(specialCharacter.getKey(), specialCharacter.getValue());
}
return format("'%s'", escaped);
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/firebolt/jdbc/type/ParserVersion.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.firebolt.jdbc.type;

public enum ParserVersion {
LEGACY, CURRENT
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,30 @@ 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
void shouldEscapeCharactersWhenTransformingFromStringLegacy() throws SQLException {
// quotes are escaped
assertEquals("'105'' OR 1=1--'' '",
JavaTypeToFireboltSQLString.STRING.transform("105' OR 1=1--' ", ParserVersion.LEGACY));
assertEquals("'105'' OR 1=1--'' '",
JavaTypeToFireboltSQLString.transformAny("105' OR 1=1--' ", ParserVersion.LEGACY));
// \0 is escaped
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\\", ParserVersion.LEGACY));
assertEquals("'105\\\\'", JavaTypeToFireboltSQLString.transformAny("105\\", ParserVersion.LEGACY));
}

@Test
Expand Down
Loading