Repository navigation
GH-1412: [JDBC] Return update counts for all statements in PreparedSt… - #1438
kaifkhan93544 wants to merge 1 commit into
Conversation
…aredStatement batch
| long totalUpdated = preparedStatement.executeUpdate(); | ||
| 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; | ||
| } |
There was a problem hiding this comment.
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.
|
|
||
| // Update query | ||
| long[] updatedCounts = {preparedStatement.executeUpdate()}; | ||
| long totalUpdated = preparedStatement.executeUpdate(); |
There was a problem hiding this comment.
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]);
}|
@kaifkhan93544 can you fix the PR title here? |
What's Changed
Fixes batch execution in
ArrowFlightMetaImplwhen executing a batch ofPreparedStatementupdates.Previously,
ArrowFlightMetaImpl.executeBatch()only returned a single-element array (long[]{ preparedStatement.executeUpdate() }), regardless of the number of parameter sets added viastmt.addBatch(). This violated the JDBC specification, which requires returning an array containing one update count for each statement in the batch.parameterValuesList.size().ArrowFlightPreparedStatementTestto assert thatexecuteBatch()returns an array with the expected length and individual update counts.Closes #1412.