You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "lidavidm (via GitHub)" <gi...@apache.org> on 2024/01/27 19:53:14 UTC
[PR] test(java): add Checker Framework [arrow-adbc]
lidavidm opened a new pull request, #1495:
URL: https://github.com/apache/arrow-adbc/pull/1495
Fixes #624.
--
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] test(java): add Checker Framework annotations [arrow-adbc]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #1495:
URL: https://github.com/apache/arrow-adbc/pull/1495#discussion_r1483590139
##########
java/driver/jdbc-validation-postgresql/src/test/java/org/apache/arrow/adbc/driver/jdbc/postgresql/PostgreSqlTypeTest.java:
##########
@@ -85,27 +85,27 @@ protected void timestamp4WithoutTimeZoneType() throws Exception {
protected void timestamp3WithoutTimeZoneType() throws Exception {
final Schema schema = connection.getTableSchema(null, null, "adbc_alltypes");
assertThat(schema.findField("timestamp_without_time_zone_p3_t").getType())
- .isEqualTo(new ArrowType.Timestamp(TimeUnit.MICROSECOND, null));
+ .isEqualTo(new ArrowType.Timestamp(TimeUnit.MILLISECOND, null));
Review Comment:
I'm not sure how this change is related to the CheckerFramework.
--
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] test(java): add Checker Framework annotations [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1495:
URL: https://github.com/apache/arrow-adbc/pull/1495#discussion_r1483594996
##########
java/driver/flight-sql/src/main/java/org/apache/arrow/adbc/driver/flightsql/InfoMetadataBuilder.java:
##########
@@ -145,7 +150,12 @@ VectorSchemaRoot build() throws AdbcException {
root.setRowCount(dstIndex);
VectorSchemaRoot result = root;
- root = null;
+ try {
+ root = VectorSchemaRoot.create(StandardSchemas.GET_INFO_SCHEMA, allocator);
Review Comment:
The class overall is AutoCloseable, and when it is closed it will clear the root. So in this method, we need to "transfer ownership" of the root out. Instead of dealing with making the root nullable, I decided to just give it an empty root always.
--
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] test(java): add Checker Framework [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #1495:
URL: https://github.com/apache/arrow-adbc/pull/1495#issuecomment-1934908867
CC @jduo @davisusanibar
--
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] test(java): add Checker Framework annotations [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1495:
URL: https://github.com/apache/arrow-adbc/pull/1495#discussion_r1483595332
##########
java/driver/jdbc-validation-postgresql/src/test/java/org/apache/arrow/adbc/driver/jdbc/postgresql/PostgreSqlTypeTest.java:
##########
@@ -85,27 +85,27 @@ protected void timestamp4WithoutTimeZoneType() throws Exception {
protected void timestamp3WithoutTimeZoneType() throws Exception {
final Schema schema = connection.getTableSchema(null, null, "adbc_alltypes");
assertThat(schema.findField("timestamp_without_time_zone_p3_t").getType())
- .isEqualTo(new ArrowType.Timestamp(TimeUnit.MICROSECOND, null));
+ .isEqualTo(new ArrowType.Timestamp(TimeUnit.MILLISECOND, null));
Review Comment:
I noticed there were two different type mapping functions and unified on the more comprehensive one.
--
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] test(java): add Checker Framework annotations [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1495:
URL: https://github.com/apache/arrow-adbc/pull/1495#discussion_r1483595536
##########
java/driver/jdbc-validation-postgresql/src/test/java/org/apache/arrow/adbc/driver/jdbc/postgresql/PostgreSqlTypeTest.java:
##########
@@ -85,27 +85,27 @@ protected void timestamp4WithoutTimeZoneType() throws Exception {
protected void timestamp3WithoutTimeZoneType() throws Exception {
final Schema schema = connection.getTableSchema(null, null, "adbc_alltypes");
assertThat(schema.findField("timestamp_without_time_zone_p3_t").getType())
- .isEqualTo(new ArrowType.Timestamp(TimeUnit.MICROSECOND, null));
+ .isEqualTo(new ArrowType.Timestamp(TimeUnit.MILLISECOND, null));
Review Comment:
Since I didn't want to null-annotate two copies of basically the same function.
--
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] test(java): add Checker Framework annotations [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #1495:
URL: https://github.com/apache/arrow-adbc/pull/1495
--
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] test(java): add Checker Framework annotations [arrow-adbc]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #1495:
URL: https://github.com/apache/arrow-adbc/pull/1495#discussion_r1483588584
##########
java/driver/flight-sql/src/main/java/org/apache/arrow/adbc/driver/flightsql/InfoMetadataBuilder.java:
##########
@@ -145,7 +150,12 @@ VectorSchemaRoot build() throws AdbcException {
root.setRowCount(dstIndex);
VectorSchemaRoot result = root;
- root = null;
+ try {
+ root = VectorSchemaRoot.create(StandardSchemas.GET_INFO_SCHEMA, allocator);
Review Comment:
Why does an empty root get recreated here?
FYI I'm reworking some of this code for the getObjects() impl. Planning to make this class an Iterator<VectorSchemaRoots> and have RootArrowReader consume the iterator to avoid front-loading the client when there are lots of metadata objects.
--
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