Skip to content

GH-1412: [JDBC] Return update counts for all statements in PreparedSt… #1438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kaifkhan93544 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

GH-1412: [JDBC] Return update counts for all statements in PreparedSt… #1438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .../flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightMetaImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.sql.SQLException;
import java.sql.SQLTimeoutException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -165,7 +166,15 @@ public ExecuteBatchResult executeBatch(
}

// Update query
long[] updatedCounts = {preparedStatement.executeUpdate()};
long totalUpdated = preparedStatement.executeUpdate();

@jbonofre jbonofre Nov 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

executeUpdate() runs before we know whether the batch is empty. If parameterValuesList is empty, nothing was bound, but the update still executes on the server. If the statement has no parameters, or still holds parameters from an earlier use, that can modify rows.
The caller then gets back an empty array and no sign it happened.

Could we return early for an empty batch, before executeUpdate() is called?

final int batchSize = parameterValuesList.size();
if (batchSize == 0) {
  return new ExecuteBatchResult(new long[0]);
}

final int batchSize = parameterValuesList.size();
long[] updatedCounts = new long[batchSize];
if (batchSize > 0) {
Arrays.fill(updatedCounts, totalUpdated / batchSize);
if (totalUpdated % batchSize != 0) {
updatedCounts[0] += totalUpdated % batchSize;
}
Comment on lines +169 to +176

@jbonofre jbonofre Nov 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

executeUpdate() returns a single aggregate count for the whole bound batch, so dividing it evenly across statements produces counts that never happened. For example, statements affecting 10, 0 and 2 rows come back as {4,4,4}. A total of 10 over 3 statements comes back as {4,3,3}, with the remainder added to the first entry. A -1 (unknown) total comes back as {-1,0,0}.

Callers that inspect individual counts, such as optimistic-locking checks that expect 0 or 1, will silently get wrong answers.

Since the per-statement counts are not known, JDBC has Statement.SUCCESS_NO_INFO for this case.
Could we fill the array with it instead?

long[] updatedCounts = new long[batchSize];
Arrays.fill(updatedCounts, Statement.SUCCESS_NO_INFO);

If there's a reason to keep the split, please add a note in Javadoc.
Even then, a negative total should not be divided.

}
return new ExecuteBatchResult(updatedCounts);
}

Expand Down
4 changes: 3 additions & 1 deletion ...dbc-core/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightPreparedStatementTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,9 @@ public void testUpdateQueryWithBatchedParameters() throws SQLException {
stmt.setArray(2, connection.createArrayOf("INTEGER", new Integer[] {0, -1, 100000}));
stmt.addBatch();
int[] updated = stmt.executeBatch();
assertEquals(42, updated[0]);
assertEquals(2, updated.length);
assertEquals(21, updated[0]);
assertEquals(21, updated[1]);
}
}
}
Loading