rzo1 commented on code in PR #184:
URL: https://github.com/apache/openjpa/pull/184#discussion_r4108000427
##########
openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/StoredProcedureQuery.java:
##########
@@ -186,15 +186,17 @@ public ResultObjectProvider executeQuery(StoreQuery q,
Object[] params, Range ra
stmnt = conn.prepareCall(_proc.getCallSQL());
final StoredProcedureQuery spq = (StoredProcedureQuery) q;
+ // no parameter values at all: leave the (un)binding to the
driver
+ final boolean bind = params != null && params.length > 0;
for (Column c : spq.getProcedure().getInColumns()) {
- if (params != null && c.getIndex() < params.length) {
+ if (bind) {
Review Comment:
Yes — the check is redundant now, and it was masking a bug.
`params` only ever comes from `toParameterArray()` in this class (via
`QueryImpl`, and `QueryCacheStoreQuery` just delegates). That method now sizes
the array to `max(index) + 1` over all IN and INOUT columns and stores each
value at `c.getIndex()`, so every index read here is in range. When nothing is
bound it returns the empty `NO_PARAM` and the new `bind` flag skips binding
altogether, which is what the length check used to do.
Previously the array was packed sequentially (all IN, then INOUT) while
`executeQuery` reads it by column position, which also counts OUT parameters.
So for a procedure like `(IN a, OUT s, IN b)`, `b` fell outside the array and
the length check silently dropped it — Derby then failed with an unrelated
error. `testInParameterAfterOutParameter` covers exactly that case.
##########
openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/StoredProcedureQuery.java:
##########
@@ -186,15 +186,17 @@ public ResultObjectProvider executeQuery(StoreQuery q,
Object[] params, Range ra
stmnt = conn.prepareCall(_proc.getCallSQL());
final StoredProcedureQuery spq = (StoredProcedureQuery) q;
+ // no parameter values at all: leave the (un)binding to the
driver
+ final boolean bind = params != null && params.length > 0;
for (Column c : spq.getProcedure().getInColumns()) {
- if (params != null && c.getIndex() < params.length) {
+ if (bind) {
Review Comment:
Yes, the check is redundant now, and it was masking a bug.
`params` only ever comes from `toParameterArray()` in this class (via
`QueryImpl`, and `QueryCacheStoreQuery` just delegates). That method now sizes
the array to `max(index) + 1` over all IN and INOUT columns and stores each
value at `c.getIndex()`, so every index read here is in range. When nothing is
bound it returns the empty `NO_PARAM` and the new `bind` flag skips binding
altogether, which is what the length check used to do.
Previously the array was packed sequentially (all IN, then INOUT) while
`executeQuery` reads it by column position, which also counts OUT parameters.
So for a procedure like `(IN a, OUT s, IN b)`, `b` fell outside the array and
the length check silently dropped it — Derby then failed with an unrelated
error. `testInParameterAfterOutParameter` covers exactly that case.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
