virajjasani commented on code in PR #2362:
URL: https://github.com/apache/phoenix/pull/2362#discussion_r2756525986


##########
phoenix-core-client/src/main/java/org/apache/phoenix/expression/util/bson/UpdateExpressionUtils.java:
##########
@@ -638,19 +640,55 @@ private static BsonValue getNewFieldValue(final BsonValue 
curValue,
         }
       }
       return getBsonNumberFromNumber(newNum);
+    } else if (curValue instanceof BsonDocument) {
+      BsonDocument doc = (BsonDocument) curValue;
+      if (doc.get("$IF_NOT_EXISTS") != null) {
+        return resolveIfNotExists(doc, bsonDocument);
+      } else if (doc.containsKey("$ADD")) {
+        BsonArray operands = doc.getArray("$ADD");
+        Number result = resolveSetOperand(operands.get(0), bsonDocument);
+        result = addNum(result, resolveSetOperand(operands.get(1), 
bsonDocument));
+        return getBsonNumberFromNumber(result);
+      } else if (doc.containsKey("$SUBTRACT")) {
+        BsonArray operands = doc.getArray("$SUBTRACT");
+        Number result = resolveSetOperand(operands.get(0), bsonDocument);
+        result = subtractNum(result, resolveSetOperand(operands.get(1), 
bsonDocument));
+        return getBsonNumberFromNumber(result);

Review Comment:
   If we need to extend `$ADD` and `$SUBTRACT` to include more operands, we can 
replace `operands.get(0)` and `operands.get(1)` within a loop. Not urgent now, 
can be done in future.



##########
phoenix-core-client/src/main/java/org/apache/phoenix/expression/util/bson/UpdateExpressionUtils.java:
##########
@@ -638,19 +640,55 @@ private static BsonValue getNewFieldValue(final BsonValue 
curValue,
         }
       }
       return getBsonNumberFromNumber(newNum);
+    } else if (curValue instanceof BsonDocument) {
+      BsonDocument doc = (BsonDocument) curValue;
+      if (doc.get("$IF_NOT_EXISTS") != null) {
+        return resolveIfNotExists(doc, bsonDocument);
+      } else if (doc.containsKey("$ADD")) {
+        BsonArray operands = doc.getArray("$ADD");
+        Number result = resolveSetOperand(operands.get(0), bsonDocument);
+        result = addNum(result, resolveSetOperand(operands.get(1), 
bsonDocument));
+        return getBsonNumberFromNumber(result);
+      } else if (doc.containsKey("$SUBTRACT")) {
+        BsonArray operands = doc.getArray("$SUBTRACT");
+        Number result = resolveSetOperand(operands.get(0), bsonDocument);
+        result = subtractNum(result, resolveSetOperand(operands.get(1), 
bsonDocument));
+        return getBsonNumberFromNumber(result);
+      }
+    }
+    return curValue;
+  }
+
+  /**
+   * Resolves an $IF_NOT_EXISTS expression. Returns the existing field value 
if present, otherwise
+   * returns the fallback value.
+   * @param ifNotExistsDoc The document containing the $IF_NOT_EXISTS 
expression
+   * @param bsonDocument   The source document to look up field values
+   * @return The resolved value (existing field value or fallback)
+   */
+  private static BsonValue resolveIfNotExists(final BsonDocument 
ifNotExistsDoc,
+    final BsonDocument bsonDocument) {
+    BsonValue ifNotExistsSpec = ifNotExistsDoc.get("$IF_NOT_EXISTS");
+    Map.Entry<String, BsonValue> ifNotExistsEntry =
+      ((BsonDocument) ifNotExistsSpec).entrySet().iterator().next();
+    String fieldPath = ifNotExistsEntry.getKey();
+    BsonValue fallbackValue = ifNotExistsEntry.getValue();
+    BsonValue existingValue =
+      CommonComparisonExpressionUtils.getFieldFromDocument(fieldPath, 
bsonDocument);
+    return (existingValue != null) ? existingValue : fallbackValue;
+  }
+
+  private static Number resolveSetOperand(BsonValue operand, BsonDocument 
bsonDocument) {
+    if (operand.isNumber()) {
+      return getNumberFromBsonNumber((BsonNumber) operand);
     } else if (
-      curValue instanceof BsonDocument && ((BsonDocument) 
curValue).get("$IF_NOT_EXISTS") != null
+      operand instanceof BsonDocument && ((BsonDocument) 
operand).get("$IF_NOT_EXISTS") != null
     ) {
-      BsonValue ifNotExistsDoc = ((BsonDocument) 
curValue).get("$IF_NOT_EXISTS");
-      Map.Entry<String, BsonValue> ifNotExistsEntry =
-        ((BsonDocument) ifNotExistsDoc).entrySet().iterator().next();
-      String ifNotExistsKey = ifNotExistsEntry.getKey();
-      BsonValue ifNotExistsVal = ifNotExistsEntry.getValue();
-      BsonValue val =
-        CommonComparisonExpressionUtils.getFieldFromDocument(ifNotExistsKey, 
bsonDocument);
-      return (val != null) ? val : ifNotExistsVal;
+      BsonValue resolved = resolveIfNotExists((BsonDocument) operand, 
bsonDocument);
+      return getNumberFromBsonNumber((BsonNumber) resolved);

Review Comment:
   If one provides non-number data type with `$IF_NOT_EXISTS`, it will fail. 
This is expected, looks good.



##########
phoenix-core-client/src/main/java/org/apache/phoenix/expression/util/bson/UpdateExpressionUtils.java:
##########
@@ -638,19 +640,55 @@ private static BsonValue getNewFieldValue(final BsonValue 
curValue,
         }
       }
       return getBsonNumberFromNumber(newNum);
+    } else if (curValue instanceof BsonDocument) {
+      BsonDocument doc = (BsonDocument) curValue;
+      if (doc.get("$IF_NOT_EXISTS") != null) {
+        return resolveIfNotExists(doc, bsonDocument);
+      } else if (doc.containsKey("$ADD")) {
+        BsonArray operands = doc.getArray("$ADD");
+        Number result = resolveSetOperand(operands.get(0), bsonDocument);
+        result = addNum(result, resolveSetOperand(operands.get(1), 
bsonDocument));
+        return getBsonNumberFromNumber(result);
+      } else if (doc.containsKey("$SUBTRACT")) {
+        BsonArray operands = doc.getArray("$SUBTRACT");
+        Number result = resolveSetOperand(operands.get(0), bsonDocument);
+        result = subtractNum(result, resolveSetOperand(operands.get(1), 
bsonDocument));
+        return getBsonNumberFromNumber(result);
+      }
+    }
+    return curValue;
+  }
+
+  /**
+   * Resolves an $IF_NOT_EXISTS expression. Returns the existing field value 
if present, otherwise
+   * returns the fallback value.
+   * @param ifNotExistsDoc The document containing the $IF_NOT_EXISTS 
expression
+   * @param bsonDocument   The source document to look up field values
+   * @return The resolved value (existing field value or fallback)
+   */
+  private static BsonValue resolveIfNotExists(final BsonDocument 
ifNotExistsDoc,
+    final BsonDocument bsonDocument) {
+    BsonValue ifNotExistsSpec = ifNotExistsDoc.get("$IF_NOT_EXISTS");
+    Map.Entry<String, BsonValue> ifNotExistsEntry =
+      ((BsonDocument) ifNotExistsSpec).entrySet().iterator().next();
+    String fieldPath = ifNotExistsEntry.getKey();
+    BsonValue fallbackValue = ifNotExistsEntry.getValue();
+    BsonValue existingValue =
+      CommonComparisonExpressionUtils.getFieldFromDocument(fieldPath, 
bsonDocument);
+    return (existingValue != null) ? existingValue : fallbackValue;
+  }
+
+  private static Number resolveSetOperand(BsonValue operand, BsonDocument 
bsonDocument) {
+    if (operand.isNumber()) {

Review Comment:
   It seems in the latest BSON version, `operand.isNumber()` seems to be fixed. 
Otherwise, we follow this pattern everywhere: `operand.isNumber() || 
operand.isDecimal128()`



-- 
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]

Reply via email to