Skip to content

Commit

Permalink
apacheGH-40249: [Java] Fix NPE in ArrowDatabaseMetadata (apache#40988)
Browse files Browse the repository at this point in the history
### Rationale for this change

When retrieving database metadata using the JDBC driver, some data such as SQL keywords could be null. Before this change, an NPE would be thrown when trying to convert the list of SQL keywords into a String.

### What changes are included in this PR?

The following database metadata fields:

* SQL keywords
* Numeric functions
* String functions
* System functions
* Time/date functions

will convert to an empty string when they are null.

### Are these changes tested?

A unit test has been added to verify that the fields above are converted to the empty string when null, with no exceptions thrown.

### Are there any user-facing changes?

The fields above will now return an empty string rather than throw an NPE.
* GitHub Issue: apache#40249

Authored-by: Norman Jordan <norman.jordan@improving.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
  • Loading branch information
normanj-bitquill authored Apr 7, 2024
1 parent 6c14172 commit 805327e
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -197,31 +198,36 @@ public boolean isReadOnly() throws SQLException {
@Override
public String getSQLKeywords() throws SQLException {
return convertListSqlInfoToString(
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_KEYWORDS, List.class));
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_KEYWORDS, List.class))
.orElse("");
}

@Override
public String getNumericFunctions() throws SQLException {
return convertListSqlInfoToString(
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_NUMERIC_FUNCTIONS, List.class));
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_NUMERIC_FUNCTIONS, List.class))
.orElse("");
}

@Override
public String getStringFunctions() throws SQLException {
return convertListSqlInfoToString(
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_STRING_FUNCTIONS, List.class));
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_STRING_FUNCTIONS, List.class))
.orElse("");
}

@Override
public String getSystemFunctions() throws SQLException {
return convertListSqlInfoToString(
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_SYSTEM_FUNCTIONS, List.class));
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_SYSTEM_FUNCTIONS, List.class))
.orElse("");
}

@Override
public String getTimeDateFunctions() throws SQLException {
return convertListSqlInfoToString(
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_DATETIME_FUNCTIONS, List.class));
getSqlInfoAndCacheIfCacheIsEmpty(SqlInfo.SQL_DATETIME_FUNCTIONS, List.class))
.orElse("");
}

@Override
Expand Down Expand Up @@ -753,8 +759,12 @@ private <T> T getSqlInfoAndCacheIfCacheIsEmpty(final SqlInfo sqlInfoCommand,
return desiredType.cast(cachedSqlInfo.get(sqlInfoCommand));
}

private String convertListSqlInfoToString(final List<?> sqlInfoList) {
return sqlInfoList.stream().map(Object::toString).collect(Collectors.joining(", "));
private Optional<String> convertListSqlInfoToString(final List<?> sqlInfoList) {
if (sqlInfoList == null) {
return Optional.empty();
} else {
return Optional.of(sqlInfoList.stream().map(Object::toString).collect(Collectors.joining(", ")));
}
}

private boolean getSqlInfoEnumOptionAndCacheIfCacheIsEmpty(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,14 @@
public class ArrowDatabaseMetadataTest {
public static final boolean EXPECTED_MAX_ROW_SIZE_INCLUDES_BLOBS = false;
private static final MockFlightSqlProducer FLIGHT_SQL_PRODUCER = new MockFlightSqlProducer();
private static final MockFlightSqlProducer FLIGHT_SQL_PRODUCER_EMPTY_SQLINFO =
new MockFlightSqlProducer();
@ClassRule
public static final FlightServerTestRule FLIGHT_SERVER_TEST_RULE = FlightServerTestRule
.createStandardTestRule(FLIGHT_SQL_PRODUCER);
@ClassRule
public static final FlightServerTestRule FLIGHT_SERVER_EMPTY_SQLINFO_TEST_RULE =
FlightServerTestRule.createStandardTestRule(FLIGHT_SQL_PRODUCER_EMPTY_SQLINFO);
private static final int ROW_COUNT = 10;
private static final List<List<Object>> EXPECTED_GET_CATALOGS_RESULTS =
range(0, ROW_COUNT)
Expand Down Expand Up @@ -604,7 +609,7 @@ public static void setUpBeforeClass() throws SQLException {

@AfterClass
public static void tearDown() throws Exception {
AutoCloseables.close(connection, FLIGHT_SQL_PRODUCER);
AutoCloseables.close(connection, FLIGHT_SQL_PRODUCER, FLIGHT_SQL_PRODUCER_EMPTY_SQLINFO);
}


Expand Down Expand Up @@ -1420,4 +1425,16 @@ public void testSqlToRegexLike() {
Assert.assertEquals("\\*", ArrowDatabaseMetadata.sqlToRegexLike("*"));
Assert.assertEquals("T\\*E.S.*T", ArrowDatabaseMetadata.sqlToRegexLike("T*E_S%T"));
}

@Test
public void testEmptySqlInfo() throws Exception {
try (final Connection testConnection = FLIGHT_SERVER_EMPTY_SQLINFO_TEST_RULE.getConnection(false)) {
final DatabaseMetaData metaData = testConnection.getMetaData();
collector.checkThat(metaData.getSQLKeywords(), is(""));
collector.checkThat(metaData.getNumericFunctions(), is(""));
collector.checkThat(metaData.getStringFunctions(), is(""));
collector.checkThat(metaData.getSystemFunctions(), is(""));
collector.checkThat(metaData.getTimeDateFunctions(), is(""));
}
}
}

0 comments on commit 805327e

Please sign in to comment.