You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "normanj-bitquill (via GitHub)" <gi...@apache.org> on 2024/04/03 20:49:05 UTC
[PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
normanj-bitquill opened a new pull request, #40988:
URL: https://github.com/apache/arrow/pull/40988
### 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.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #40988:
URL: https://github.com/apache/arrow/pull/40988#discussion_r1550908906
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowDatabaseMetadata.java:
##########
@@ -754,7 +754,11 @@ private <T> T getSqlInfoAndCacheIfCacheIsEmpty(final SqlInfo sqlInfoCommand,
}
private String convertListSqlInfoToString(final List<?> sqlInfoList) {
Review Comment:
I think it would make more sense to return `Optional<String>` and update client code to explicitly decide what they want to do with missing values.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40988:
URL: https://github.com/apache/arrow/pull/40988#discussion_r1550594596
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowDatabaseMetadata.java:
##########
@@ -754,7 +754,11 @@ private <T> T getSqlInfoAndCacheIfCacheIsEmpty(final SqlInfo sqlInfoCommand,
}
private String convertListSqlInfoToString(final List<?> sqlInfoList) {
- return sqlInfoList.stream().map(Object::toString).collect(Collectors.joining(", "));
+ if (sqlInfoList == null) {
Review Comment:
Could the items in the list be null?
Should we explicitly handle it?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #40988:
URL: https://github.com/apache/arrow/pull/40988#discussion_r1553957641
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowDatabaseMetadata.java:
##########
@@ -754,7 +754,11 @@ private <T> T getSqlInfoAndCacheIfCacheIsEmpty(final SqlInfo sqlInfoCommand,
}
private String convertListSqlInfoToString(final List<?> sqlInfoList) {
Review Comment:
Updated the return type of `convertListSqlInfoToString()` to a `Optional<String>`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #40988:
URL: https://github.com/apache/arrow/pull/40988#discussion_r1551960883
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowDatabaseMetadata.java:
##########
@@ -754,7 +754,11 @@ private <T> T getSqlInfoAndCacheIfCacheIsEmpty(final SqlInfo sqlInfoCommand,
}
private String convertListSqlInfoToString(final List<?> sqlInfoList) {
Review Comment:
This method is used by these methods:
* getSqlKeywords()
* getNumericFunctions()
* getStringFunctions()
* getSystemFunctions()
* getTimeDateFunctions()
These methods come from Avatica, so we can't easily change the return type.
https://javadoc.io/static/org.apache.calcite.avatica/avatica-core/1.24.0/org/apache/calcite/avatica/AvaticaDatabaseMetaData.html#getSQLKeywords--
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on PR #40988:
URL: https://github.com/apache/arrow/pull/40988#issuecomment-2041042073
@github-actions crossbow submit *java*
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #40988:
URL: https://github.com/apache/arrow/pull/40988#discussion_r1552519380
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowDatabaseMetadata.java:
##########
@@ -754,7 +754,11 @@ private <T> T getSqlInfoAndCacheIfCacheIsEmpty(final SqlInfo sqlInfoCommand,
}
private String convertListSqlInfoToString(final List<?> sqlInfoList) {
Review Comment:
But we can update our implementations to be explicit, right?
In any case, defaulting to an empty string also seems fine so long as it's documented in the helper's docstring
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40988:
URL: https://github.com/apache/arrow/pull/40988#issuecomment-2035560356
:warning: GitHub issue #40249 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40988:
URL: https://github.com/apache/arrow/pull/40988#issuecomment-2041042826
Revision: 2a51fa6e005f6acd5eb2ecc12b75c50d0a79d84d
Submitted crossbow builds: [ursacomputing/crossbow @ actions-f7fbc36bed](https://github.com/ursacomputing/crossbow/branches/all?query=actions-f7fbc36bed)
|Task|Status|
|----|------|
|java-jars|[](https://github.com/ursacomputing/crossbow/actions/runs/8580527178/job/23516677478)|
|verify-rc-source-java-linux-almalinux-8-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/8580527162/job/23516677044)|
|verify-rc-source-java-linux-conda-latest-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/8580527245/job/23516677174)|
|verify-rc-source-java-linux-ubuntu-20.04-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/8580527176/job/23516677048)|
|verify-rc-source-java-linux-ubuntu-22.04-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/8580527294/job/23516677271)|
|verify-rc-source-java-macos-amd64|[](https://github.com/ursacomputing/crossbow/tree/actions-f7fbc36bed-github-verify-rc-source-java-macos-amd64)|
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "normanj-bitquill (via GitHub)" <gi...@apache.org>.
normanj-bitquill commented on code in PR #40988:
URL: https://github.com/apache/arrow/pull/40988#discussion_r1552090765
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowDatabaseMetadata.java:
##########
@@ -754,7 +754,11 @@ private <T> T getSqlInfoAndCacheIfCacheIsEmpty(final SqlInfo sqlInfoCommand,
}
private String convertListSqlInfoToString(final List<?> sqlInfoList) {
- return sqlInfoList.stream().map(Object::toString).collect(Collectors.joining(", "));
+ if (sqlInfoList == null) {
Review Comment:
@vibhatha The values cannot be null.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #40988:
URL: https://github.com/apache/arrow/pull/40988
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40249: [Java] Fix NPE in ArrowDatabaseMetadata [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #40988:
URL: https://github.com/apache/arrow/pull/40988#issuecomment-2042902907
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 805327e48d81cc1ccd3ae4de749547c4f1f4842c.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/23569996481) has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org