laurentgo commented on code in PR #1335:
URL: https://github.com/apache/arrow-java/pull/1335#discussion_r4241240369
##########
flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightPreparedStatementTest.java:
##########
@@ -259,6 +260,29 @@ public void testReturnColumnCount() throws SQLException {
}
}
+ @Test
+ public void testReturnColumnNullability() throws SQLException {
Review Comment:
I am wondering if we need a new test case or update an existing query as
some of the existing columns while being marked nullable do not contain
nullable data, and add the nullability check in one of the existing tests above
##########
flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightPreparedStatementTest.java:
##########
@@ -259,6 +260,29 @@ public void testReturnColumnCount() throws SQLException {
}
}
+ @Test
+ public void testReturnColumnNullability() throws SQLException {
+ final String query = "SELECT nullable, required FROM nullability_test";
+ final Schema schema =
+ new Schema(
+ Arrays.asList(
+ Field.nullable("nullable", ArrowType.Utf8.INSTANCE),
+ Field.notNullable("required", ArrowType.Utf8.INSTANCE)));
+ PRODUCER.addSelectQuery(query, schema, Collections.emptyList());
+ try (final PreparedStatement psmt = connection.prepareStatement(query)) {
+ assertAll(
+ "Column nullability is as expected",
+ () -> assertThat("nullable",
equalTo(psmt.getMetaData().getColumnName(1))),
Review Comment:
(nit) evaluation order should be swapped
(`psmt.getMetadata().[...].equalsTo("nullable")` in order for the description
to be accurate in case of an error.
Same for other assertions below
##########
flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/utils/ConvertUtilsTest.java:
##########
@@ -104,6 +106,7 @@ public void
testShouldConvertArrowFieldsToColumnMetaDataList() {
.setSchemaName("schema1")
.setTableName("table1")
.setColumnName("col2")
+ .setNullable(ResultSetMetaData.columnNullable)
Review Comment:
Maybe `col2` nullability should be changed so both paths are being tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
