FIR-32015: implemented missing trivial setters in PreparedStatement (#…

…390)
firebolt-db · Apr 24, 2024 · 37a0f1c · 37a0f1c
1 parent fd273fc
commit 37a0f1c
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 52 deletions.
diff --git a/src/integrationTest/java/integration/tests/PreparedStatementTest.java b/src/integrationTest/java/integration/tests/PreparedStatementTest.java
@@ -1,5 +1,7 @@
 package integration.tests;
 
+import com.firebolt.jdbc.CheckedBiFunction;
+import com.firebolt.jdbc.CheckedTriFunction;
 import com.firebolt.jdbc.QueryResult;
 import com.firebolt.jdbc.resultset.FireboltResultSet;
 import com.firebolt.jdbc.testutils.AssertionUtil;
@@ -13,13 +15,18 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import javax.sql.rowset.serial.SerialBlob;
 import javax.sql.rowset.serial.SerialClob;
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.StringReader;
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
@@ -32,11 +39,13 @@
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static java.sql.Statement.SUCCESS_NO_INFO;
 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.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 @CustomLog
@@ -85,6 +94,59 @@ void shouldInsertRecordsInBatch() throws SQLException {
 		}
 	}
 
+	Stream<Arguments> numericTypes() {
+		return Stream.of(
+				Arguments.of("byte",
+						(CheckedTriFunction<PreparedStatement, Integer, Integer, Void>) (s, i, v) -> {
+							s.setByte(i, v.byteValue());
+							return null;
+						}, (CheckedBiFunction<ResultSet, Integer, Number>) (rs, i) -> (int) rs.getByte(i)),
+
+				Arguments.of("short",
+						(CheckedTriFunction<PreparedStatement, Integer, Integer, Void>) (s, i, v) -> {
+							s.setShort(i, v.shortValue());
+							return null;
+						}, (CheckedBiFunction<ResultSet, Integer, Number>) (rs, i) -> (int) rs.getShort(i)),
+
+				Arguments.of("int",
+						(CheckedTriFunction<PreparedStatement, Integer, Integer, Void>) (s, i, v) -> {
+							s.setInt(i, v);
+							return null;
+						}, (CheckedBiFunction<ResultSet, Integer, Number>) (rs, i) -> (int) rs.getInt(i)),
+
+				Arguments.of("long",
+						(CheckedTriFunction<PreparedStatement, Integer, Integer, Void>) (s, i, v) -> {
+							s.setLong(i, v.longValue());
+							return null;
+						}, (CheckedBiFunction<ResultSet, Integer, Number>) (rs, i) -> (int) rs.getLong(i))
+		);
+	}
+
+	@ParameterizedTest(name = "{0}")
+	@MethodSource("numericTypes")
+	<T> void shouldInsertRecordsUsingDifferentNumericTypes(String name, CheckedTriFunction<PreparedStatement, Integer, Integer, Void> setter, CheckedBiFunction<ResultSet, Integer, T> getter) throws SQLException {
+		Car car = Car.builder().make("Tesla").sales(42).build();
+		try (Connection connection = createConnection()) {
+
+			try (PreparedStatement statement = connection
+					.prepareStatement("INSERT INTO prepared_statement_test (sales, make) VALUES (?,?)")) {
+				setter.apply(statement, 1, car.getSales());
+				statement.setString(2, car.getMake());
+				statement.executeUpdate();
+			}
+
+			try (Statement statement = connection.createStatement();
+				 ResultSet rs = statement.executeQuery("SELECT sales, make FROM prepared_statement_test ORDER BY make")) {
+				assertTrue(rs.next());
+				assertEquals(car.getSales(), getter.apply(rs, 1));
+				assertEquals(car.getMake(), rs.getString(2));
+				rs.getString(2);
+
+				assertFalse(rs.next());
+			}
+		}
+	}
+
 	@Test
 	void shouldReplaceParamMarkers() throws SQLException {
 		String insertSql = "INSERT INTO prepared_statement_test(sales, make) VALUES /* Some comment ? */ -- other comment ? \n  (?,?)";
@@ -288,6 +350,34 @@ private QueryResult createExpectedResult(List<List<?>> expectedRows) {
 
 	}
 
+	@Test
+	void shouldInsertAndRetrieveUrl() throws SQLException, MalformedURLException {
+		Car tesla = Car.builder().make("https://www.tesla.com/").sales(300).build();
+		Car nothing = Car.builder().sales(0).build();
+		try (Connection connection = createConnection()) {
+			try (PreparedStatement statement = connection
+					.prepareStatement("INSERT INTO prepared_statement_test (sales, make) VALUES (?,?)")) {
+				statement.setInt(1, tesla.getSales());
+				statement.setURL(2, new URL(tesla.getMake()));
+				statement.executeUpdate();
+				statement.setInt(1, nothing.getSales());
+				statement.setURL(2, null);
+				statement.executeUpdate();
+			}
+
+			try (Statement statement = connection.createStatement();
+				 ResultSet rs = statement.executeQuery("SELECT make FROM prepared_statement_test order by sales")) {
+				assertTrue(rs.next());
+				assertNull(rs.getString(1));
+				assertNull(rs.getURL(1));
+				assertTrue(rs.next());
+				assertEquals("https://www.tesla.com/", rs.getString(1));
+				assertEquals(new URL("https://www.tesla.com/"), rs.getURL(1));
+				assertFalse(rs.next());
+			}
+		}
+	}
+
 	@Builder
 	@Value
 	@EqualsAndHashCode

diff --git a/src/integrationTest/resources/statements/prepared-statement/ddl.sql b/src/integrationTest/resources/statements/prepared-statement/ddl.sql
@@ -1,7 +1,7 @@
 DROP TABLE IF EXISTS prepared_statement_test;
 CREATE
 FACT TABLE IF NOT EXISTS prepared_statement_test (
-make            STRING not null,
+make            STRING null,
 sales           bigint not null,
 signature       bytea null
 )

diff --git a/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java b/src/main/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatement.java
@@ -104,7 +104,9 @@ public void setBoolean(int parameterIndex, boolean x) throws SQLException {
 
 	@Override
 	public void setByte(int parameterIndex, byte x) throws SQLException {
-		throw new FireboltSQLFeatureNotSupportedException();
+		validateStatementIsNotClosed();
+		validateParamIndex(parameterIndex);
+		providedParameters.put(parameterIndex, JavaTypeToFireboltSQLString.BYTE.transform(x));
 	}
 
 	@Override
@@ -238,9 +240,8 @@ public void setNull(int parameterIndex, int sqlType, String typeName) throws SQL
 	}
 
 	@Override
-	@NotImplemented
-	public void setURL(int parameterIndex, URL x) throws SQLException {
-		throw new FireboltSQLFeatureNotSupportedException();
+	public void setURL(int parameterIndex, URL url) throws SQLException {
+		setString(parameterIndex, url == null ? null : url.toString());
 	}
 
 	@Override

diff --git a/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java b/src/main/java/com/firebolt/jdbc/type/JavaTypeToFireboltSQLString.java
@@ -31,6 +31,7 @@
 public enum JavaTypeToFireboltSQLString {
 	BOOLEAN(Boolean.class, value -> Boolean.TRUE.equals(value) ? "1" : "0"),
 	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()),
 	LONG(Long.class, value -> Long.toString(((Number)value).longValue())),

diff --git a/...est/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatementTest.java b/...est/java/com/firebolt/jdbc/statement/preparedstatement/FireboltPreparedStatementTest.java
@@ -82,16 +82,19 @@ private interface Setter {
 		void set(PreparedStatement ps) throws SQLException;
 	}
 
+	private interface ParameterizedSetter<T> {
+		void set(PreparedStatement statement, int index, T value) throws SQLException;
+	}
+
 	private static class SerialNClob extends SerialClob implements NClob {
 		public SerialNClob(char[] ch) throws SQLException {
 			super(ch);
 		}
 	}
 
+
 	private static Stream<Arguments> unsupported() {
 		return Stream.of(
-					Arguments.of("setByte", (Executable) () -> statement.setByte(1, (byte) 127)),
-					Arguments.of("setURL", (Executable) () -> statement.setURL(1, new URL("http://foo.bar"))),
 					Arguments.of("setRef", (Executable) () -> statement.setRef(1, mock(Ref.class))),
 					Arguments.of("setDate", (Executable) () -> statement.setDate(1, new Date(System.currentTimeMillis()), Calendar.getInstance())),
 					Arguments.of("setTime", (Executable) () -> statement.setTime(1, new Time(System.currentTimeMillis()))),
@@ -184,7 +187,18 @@ private static Stream<Arguments> buffers() {
 				Arguments.of("setUnicodeStream(InputStream, length+1)", (Setter) statement -> statement.setUnicodeStream(1, new ByteArrayInputStream("hello".getBytes()), 6), "E'\\x68\\x65\\x6c\\x6c\\x6f'::BYTEA"),
 				Arguments.of("setUnicodeStream(InputStream, 42)", (Setter) statement -> statement.setUnicodeStream(1, new ByteArrayInputStream("hello".getBytes()), 42), "E'\\x68\\x65\\x6c\\x6c\\x6f'::BYTEA"),
 				Arguments.of("setUnicodeStream(InputStream, 1)", (Setter) statement -> statement.setUnicodeStream(1, new ByteArrayInputStream("hello".getBytes()), 1), "E'\\x68'::BYTEA"),
-				Arguments.of("setUnicodeStream(InputStream, 0)", (Setter) statement -> statement.setUnicodeStream(1, new ByteArrayInputStream("hello".getBytes()), 0), "E'\\x'::BYTEA")
+				Arguments.of("setUnicodeStream(InputStream, 0)", (Setter) statement -> statement.setUnicodeStream(1, new ByteArrayInputStream("hello".getBytes()), 0), "E'\\x'::BYTEA"));
+	}
+
+	private static Stream<Arguments> setNumber() {
+		return Stream.of(
+				Arguments.of("byte", (ParameterizedSetter<Byte>) PreparedStatement::setByte, (byte)50),
+				Arguments.of("short", (ParameterizedSetter<Short>) PreparedStatement::setShort, (short)50),
+				Arguments.of("int", (ParameterizedSetter<Integer>) PreparedStatement::setInt, 50),
+				Arguments.of("long", (ParameterizedSetter<Long>) PreparedStatement::setLong, 50L),
+				Arguments.of("float", (ParameterizedSetter<Float>) PreparedStatement::setFloat, 5.5f),
+				Arguments.of("double", (ParameterizedSetter<Double>) PreparedStatement::setDouble, 3.14),
+				Arguments.of("double", (ParameterizedSetter<BigDecimal>) PreparedStatement::setBigDecimal, new BigDecimal("555555555555.55555555"))
 		);
 	}
 
@@ -358,65 +372,42 @@ void shouldSetBoolean() throws SQLException {
 		assertEquals("INSERT INTO cars(available) VALUES (1)", queryInfoWrapperArgumentCaptor.getValue().getSql());
 	}
 
-	@ParameterizedTest(name = "{0}")
-	@MethodSource("unsupported")
-	void shouldThrowSQLFeatureNotSupportedException(String name, Executable function) {
-		statement = createStatementWithSql("INSERT INTO cars(make) VALUES (?)");
-		assertThrows(SQLFeatureNotSupportedException.class, function);
-	}
-
-	@Test
-	void shouldSetInt() throws SQLException {
-		statement = createStatementWithSql("INSERT INTO cars(price) VALUES (?)");
-
-		statement.setInt(1, 50);
-		statement.execute();
-
-		verify(fireboltStatementService).execute(queryInfoWrapperArgumentCaptor.capture(), eq(properties), anyBoolean(), any());
-	}
-
-	@Test
-	void shouldSetLong() throws SQLException {
-		statement = createStatementWithSql("INSERT INTO cars(price) VALUES (?)");
+	@ParameterizedTest
+	@CsvSource(value = {
+			"Nobody,",
+			"Firebolt,http://www.firebolt.io"
+	})
+	void shouldSetUrl(String name, URL url) throws SQLException {
+		statement = createStatementWithSql("INSERT INTO companies (name, url) VALUES (?,?)");
 
-		statement.setLong(1, 50L);
+		statement.setString(1, name);
+		statement.setURL(2, url);
 		statement.execute();
 
 		verify(fireboltStatementService).execute(queryInfoWrapperArgumentCaptor.capture(), eq(properties), anyBoolean(), any());
-		assertEquals("INSERT INTO cars(price) VALUES (50)", queryInfoWrapperArgumentCaptor.getValue().getSql());
+		assertEquals(format("INSERT INTO companies (name, url) VALUES (%s,%s)", sqlQuote(name), sqlQuote(url)), queryInfoWrapperArgumentCaptor.getValue().getSql());
 	}
 
-	@Test
-	void shouldSetFloat() throws SQLException {
-		statement = createStatementWithSql("INSERT INTO cars(price) VALUES (?)");
-
-		statement.setFloat(1, 5.5F);
-		statement.execute();
-
-		verify(fireboltStatementService).execute(queryInfoWrapperArgumentCaptor.capture(), eq(properties), anyBoolean(), any());
-		assertEquals("INSERT INTO cars(price) VALUES (5.5)", queryInfoWrapperArgumentCaptor.getValue().getSql());
+	private String sqlQuote(Object value) {
+		return value == null ? "NULL" : format("'%s'", value);
 	}
 
-	@Test
-	void shouldSetDouble() throws SQLException {
-		statement = createStatementWithSql("INSERT INTO cars(price) VALUES (?)");
-
-		statement.setDouble(1, 5.5);
-		statement.execute();
-
-		verify(fireboltStatementService).execute(queryInfoWrapperArgumentCaptor.capture(), eq(properties), anyBoolean(), any());
+	@ParameterizedTest(name = "{0}")
+	@MethodSource("unsupported")
+	void shouldThrowSQLFeatureNotSupportedException(String name, Executable function) {
+		statement = createStatementWithSql("INSERT INTO cars(make) VALUES (?)");
+		assertThrows(SQLFeatureNotSupportedException.class, function);
 	}
 
-	@Test
-	void shouldSetBigDecimal() throws SQLException {
+	@ParameterizedTest(name = "{0}")
+	@MethodSource("setNumber")
+	<T> void shouldSetNumber(String name, ParameterizedSetter<T> setter, T value) throws SQLException {
 		statement = createStatementWithSql("INSERT INTO cars(price) VALUES (?)");
 
-		statement.setBigDecimal(1, new BigDecimal("555555555555.55555555"));
+		setter.set(statement, 1, value);
 		statement.execute();
 
 		verify(fireboltStatementService).execute(queryInfoWrapperArgumentCaptor.capture(), eq(properties), anyBoolean(), any());
-		assertEquals("INSERT INTO cars(price) VALUES (555555555555.55555555)",
-				queryInfoWrapperArgumentCaptor.getValue().getSql());
 	}
 
 	@Test