haridsv commented on code in PR #2199:
URL: https://github.com/apache/phoenix/pull/2199#discussion_r2163491415
##########
phoenix-core-server/src/main/java/org/apache/phoenix/hbase/index/IndexRegionObserver.java:
##########
@@ -1892,6 +1911,13 @@ private List<Mutation>
generateOnDupMutations(BatchMutateContext context,
Pair<Put, Put> dataRowState = context.dataRowStates.get(rowKeyPtr);
Put currentDataRowState = dataRowState != null ? dataRowState.getFirst()
: null;
Review Comment:
The above 4 lines seem to have incorrect indent level?
##########
phoenix-core-server/src/main/java/org/apache/phoenix/hbase/index/IndexRegionObserver.java:
##########
@@ -1316,6 +1320,11 @@ private void
identifyMutationTypes(MiniBatchOperationInProgress<Mutation> miniBa
Mutation m = miniBatchOp.getOperation(i);
if (this.builder.returnResult(m) && miniBatchOp.size() == 1) {
context.returnResult = true;
+ byte[] returnResult =
m.getAttribute(PhoenixIndexBuilderHelper.RETURN_RESULT);
+ if (returnResult != null && Arrays.equals(returnResult,
+ PhoenixIndexBuilderHelper.RETURN_RESULT_OLD_ROW)) {
Review Comment:
Considering there is existing build.returnResult(), why not add one more
returnOldResult() and use it?
I also noticed an existing usage at line 798 that I think can be replaced
with builder.returnResult() like this:
```
---
phoenix-core-server/src/main/java/org/apache/phoenix/hbase/index/IndexRegionObserver.java
+++
phoenix-core-server/src/main/java/org/apache/phoenix/hbase/index/IndexRegionObserver.java
@@ -1864,8 +1864,7 @@ public class IndexRegionObserver implements
RegionCoprocessor, RegionObserver {
throws IOException {
List<Mutation> mutations = Lists.newArrayListWithExpectedSize(2);
byte[] opBytes = atomicPut.getAttribute(ATOMIC_OP_ATTRIB);
- byte[] returnResult = atomicPut.getAttribute(RETURN_RESULT);
- if ((opBytes == null && returnResult == null) ||
+ if ((opBytes == null && this.builder.returnResult(atomicPut)) ||
(opBytes == null && miniBatchOp.size() != 1)) {
// Unexpected
// Either mutation should be atomic by providing non-null ON
DUPLICATE KEY, or
```
--
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: issues-unsubscr...@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
