haridsv commented on code in PR #2226:
URL: https://github.com/apache/phoenix/pull/2226#discussion_r2257069133
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/OnDuplicateKey2IT.java:
##########
@@ -591,8 +598,8 @@ private static void
validateReturnedRowBeforeUpsert(Connection conn, String upse
updateCount = resultPair.getFirst();
resultSet = resultPair.getSecond();
}
- boolean isOnDuplicateKey = upsertSql.toUpperCase().contains("ON DUPLICATE
KEY");
- if (conn.getAutoCommit() && isOnDuplicateKey) {
+ boolean isReturningRow = upsertSql.toUpperCase().contains("RETURNING *");
Review Comment:
> That is correct. However, the problem here is that some tests using this
method don't have RETURNING * but we are testing
executeAtomicUpdateReturnOldRow() which will anyways return row.
This is to facilitate the following variations:
- Statement without `RETURNING *` executed via standard JDBC. In this case
no, there will be no result set.
- Statement with `RETURNING *` executed via standard JDBC. This results in a
result set that will confirm to the JDBC standard, i.e., client would need to
call `next()` before accessing the (first and only) row.
- Statement with or without `RETURNING *` executed via one of
executeAtomicUpdateReturn* methods. This results in a result set that is
prefetched, i.e., `next()` has already been called. This maintains backward
compatibility with the existing behavior.
The reason for the check to support the 1st case above when the test is
executing statements that don't return a RS and these are existing test cases.
--
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]
