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 2026/06/04 01:21:13 UTC
[PR] feat(java/driver/jni): implement binding of streams [arrow-adbc]
lidavidm opened a new pull request, #4359:
URL: https://github.com/apache/arrow-adbc/pull/4359
Depends on #4357.
Closes #4358.
--
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] feat(java/driver/jni): implement binding of streams [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #4359:
URL: https://github.com/apache/arrow-adbc/pull/4359
--
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] feat(java/driver/jni): implement binding of streams [arrow-adbc]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4359:
URL: https://github.com/apache/arrow-adbc/pull/4359#discussion_r3353388181
##########
java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/JniStatement.java:
##########
@@ -53,23 +59,48 @@ public void setSqlQuery(String query) throws AdbcException {
@Override
public void bind(VectorSchemaRoot root) throws AdbcException {
+ clearBind();
this.bindRoot = root;
}
+ @Override
+ public void bind(ArrowReader reader) throws AdbcException {
+ clearBind();
+ this.bindRoot = null;
+ this.bindStream = reader;
+ }
+
+ private void clearBind() throws AdbcException {
+ if (this.bindStream != null) {
+ try {
+ this.bindStream.close();
+ } catch (IOException e) {
+ throw AdbcException.internal("[jni] failed to close previous bind stream").withCause(e);
+ } finally {
+ this.bindStream = null;
+ }
+ }
+ this.bindRoot = null;
+ }
+
// The C Data export takes ownership of the data at bind time and ignores subsequent
// client changes to the bound root. Defer the export until execution so we capture
// the final state of the VectorSchemaRoot.
private void exportBind() throws AdbcException {
- if (bindRoot == null) {
- return;
- }
- try (final ArrowArray batch = ArrowArray.allocateNew(allocator);
- final ArrowSchema schema = ArrowSchema.allocateNew(allocator)) {
- // TODO(lidavidm): we may need a way to separately provide a dictionary provider
- Data.exportSchema(allocator, bindRoot.getSchema(), null, schema);
- Data.exportVectorSchemaRoot(allocator, bindRoot, null, batch);
-
- JniLoader.INSTANCE.statementBind(handle, batch, schema);
+ if (bindRoot != null) {
+ try (final ArrowArray batch = ArrowArray.allocateNew(allocator);
+ final ArrowSchema schema = ArrowSchema.allocateNew(allocator)) {
+ // TODO(lidavidm): we may need a way to separately provide a dictionary provider
+ Data.exportSchema(allocator, bindRoot.getSchema(), null, schema);
+ Data.exportVectorSchemaRoot(allocator, bindRoot, null, batch);
+
+ JniLoader.INSTANCE.statementBind(handle, batch, schema);
+ }
+ } else if (bindStream != null) {
+ try (final ArrowArrayStream stream = ArrowArrayStream.allocateNew(allocator)) {
+ Data.exportArrayStream(allocator, bindStream, stream);
+ JniLoader.INSTANCE.statementBindStream(handle, stream);
+ }
Review Comment:
After exporting/binding an ArrowReader via AdbcStatementBindStream, the C driver takes ownership of the exported ArrowArrayStream and will invoke its release callback (which will close the underlying ArrowReader). Keeping `bindStream` set here means `clearBind()`/`close()` may later close the same reader again, or close it before the native statement releases it, violating the ownership contract of `AdbcStatementBindStream` (see adbc.h:2114-2116). Clear the Java reference once the bind succeeds so the statement no longer attempts to manage the reader lifecycle after ownership is transferred.
##########
java/driver/jni-validation/src/test/java/org/apache/arrow/adbc/driver/jni/PostgresIntegrationTest.java:
##########
@@ -569,6 +573,37 @@ void bindUpdateQuery() throws Exception {
}
}
+ @Test
+ void bindStream() throws Exception {
+ // Create temp Arrow file to get an ArrowReader
+ Schema schema = new Schema(List.of(Field.nullable("i", Types.MinorType.INT.getType())));
Review Comment:
For stream binding, other tests in this file use parameter-style field names (e.g. "$1" in bindSelectQuery). If this test is updated to actually execute a statement (which it should), the stream schema should match that convention so the driver can map columns to bind parameters consistently.
##########
java/driver/jni-validation/src/test/java/org/apache/arrow/adbc/driver/jni/PostgresIntegrationTest.java:
##########
@@ -569,6 +573,37 @@ void bindUpdateQuery() throws Exception {
}
}
+ @Test
+ void bindStream() throws Exception {
+ // Create temp Arrow file to get an ArrowReader
+ Schema schema = new Schema(List.of(Field.nullable("i", Types.MinorType.INT.getType())));
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ try (VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator);
+ ArrowStreamWriter writer = new ArrowStreamWriter(root, null, baos)) {
+ writer.start();
+
+ IntVector iv = (IntVector) root.getVector(0);
+
+ iv.setSafe(0, 1);
+ iv.setSafe(1, 42);
+ iv.setNull(2);
+ root.setRowCount(3);
+ writer.writeBatch();
+
+ iv.setSafe(0, 10);
+ iv.setSafe(1, 20);
+ iv.setSafe(2, 30);
+ root.setRowCount(3);
+ writer.writeBatch();
+ }
+
+ ArrowStreamReader reader =
+ new ArrowStreamReader(new ByteArrayInputStream(baos.toByteArray()), allocator);
+ try (var stmt = conn.createStatement()) {
+ stmt.bind(reader);
+ }
Review Comment:
This test currently never executes a query/update after binding the ArrowReader, so it doesn't exercise the new `statementBindStream` JNI path (exporting the ArrowReader to C and letting the driver consume it). Execute a simple bound query and assert results so regressions in stream binding are caught.
##########
java/driver/jni-validation/src/test/java/org/apache/arrow/adbc/driver/jni/SqlServerIntegrationTest.java:
##########
@@ -598,6 +602,37 @@ void bindUpdateQuery() throws Exception {
}
}
+ @Test
+ void bindStream() throws Exception {
+ // Create temp Arrow file to get an ArrowReader
+ Schema schema = new Schema(List.of(Field.nullable("i", Types.MinorType.INT.getType())));
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ try (VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator);
+ ArrowStreamWriter writer = new ArrowStreamWriter(root, null, baos)) {
+ writer.start();
+
+ IntVector iv = (IntVector) root.getVector(0);
+
+ iv.setSafe(0, 1);
+ iv.setSafe(1, 42);
+ iv.setNull(2);
+ root.setRowCount(3);
+ writer.writeBatch();
+
+ iv.setSafe(0, 10);
+ iv.setSafe(1, 20);
+ iv.setSafe(2, 30);
+ root.setRowCount(3);
+ writer.writeBatch();
+ }
+
+ ArrowStreamReader reader =
+ new ArrowStreamReader(new ByteArrayInputStream(baos.toByteArray()), allocator);
+ try (var stmt = conn.createStatement()) {
+ stmt.bind(reader);
+ }
Review Comment:
This test only binds the ArrowReader and closes the statement; it never executes anything, so it doesn't exercise the new JNI stream-binding/export path. Execute a simple bound query (similar to bindSelectQuery) and assert the results so stream binding failures are caught.
##########
java/driver/jni-validation/src/test/java/org/apache/arrow/adbc/driver/jni/SqlServerIntegrationTest.java:
##########
@@ -598,6 +602,37 @@ void bindUpdateQuery() throws Exception {
}
}
+ @Test
+ void bindStream() throws Exception {
+ // Create temp Arrow file to get an ArrowReader
+ Schema schema = new Schema(List.of(Field.nullable("i", Types.MinorType.INT.getType())));
Review Comment:
To meaningfully test stream binding (i.e., execute a statement using the bound values), the stream schema should follow the parameter-style naming used elsewhere in this file (e.g., "$1" in bindSelectQuery) so the driver can map stream columns to bind parameters consistently.
--
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