Skip to content

PHOENIX-7880 : Do not treat literal string SET values as arithmetic in BSON UpdateExpression (#2496) #2498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
palashc wants to merge 1 commit into
base: 5.3
Choose a base branch
from
Open

PHOENIX-7880 : Do not treat literal string SET values as arithmetic in BSON UpdateExpression (#2496) #2498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 15 additions & 94 deletions ...e-client/src/main/java/org/apache/phoenix/expression/util/bson/UpdateExpressionUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,10 @@
package org.apache.phoenix.expression.util.bson;

import java.math.BigDecimal;
import java.text.NumberFormat;
import java.text.ParseException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.bson.BsonArray;
import org.bson.BsonDecimal128;
import org.bson.BsonDocument;
Expand Down Expand Up @@ -102,7 +98,7 @@ private enum UpdateOp {
public static void updateExpression(final BsonDocument updateExpression,
final BsonDocument bsonDocument) {

LOGGER.info("Update Expression: {} , current bsonDocument: {}", updateExpression, bsonDocument);
LOGGER.debug("Update Expression: {} , current bsonDocument: {}", updateExpression, bsonDocument);

if (updateExpression.containsKey("$SET")) {
executeSetExpression((BsonDocument) updateExpression.get("$SET"), bsonDocument);
Expand Down Expand Up @@ -578,73 +574,26 @@ private static void updateDocumentAtLeafNode(BsonValue newVal, UpdateOp updateOp
}

/**
* Retrieve the value to be updated for the given current value. If the current value does not
* contain any arithmetic operators, the current value is returned without any modifications. If
* the current value contains arithmetic expressions like "a + b" or "a - b", the values of
* operands are retrieved from the given document and if the values are numeric, the given
* arithmetic operation is performed. If the current value is a bson document with an entry from
* $IF_NOT_EXISTS to a document with a key and a fallback value, we lookup if the key is already
* present in the document. If it is, we return its value. Otherwise, we return the provided
* fallback value. If the current value is a bson document with $ADD or $SUBTRACT as key, we get
* the array of operands from this document and perform the corresponding operation. Operand can
* be an $IF_NOT_EXISTS bson document. If the current value is a bson document with $LIST_APPEND
* as key, the value is a two-element array of operands; each operand resolves to a BsonArray
* (literal array, a path string referring to an existing array attribute, or an $IF_NOT_EXISTS
* document whose resolved value is an array) and the two arrays are concatenated in order with
* duplicates preserved.
* Retrieve the value to be updated for the given current value. Arithmetic and other computed SET
* values are always carried as explicit BSON documents (never inferred from the textual content
* of a string value), so any non-document value - including a {@link BsonString} that happens to
* contain " + " or " - " - is treated as a literal and returned without modification. If the
* current value is a bson document with an entry from $IF_NOT_EXISTS to a document with a key and
* a fallback value, we lookup if the key is already present in the document. If it is, we return
* its value. Otherwise, we return the provided fallback value. If the current value is a bson
* document with $ADD or $SUBTRACT as key, we get the array of operands from this document and
* perform the corresponding operation. Operand can be an $IF_NOT_EXISTS bson document. If the
* current value is a bson document with $LIST_APPEND as key, the value is a two-element array of
* operands; each operand resolves to a BsonArray (literal array, a path string referring to an
* existing array attribute, or an $IF_NOT_EXISTS document whose resolved value is an array) and
* the two arrays are concatenated in order with duplicates preserved.
* @param curValue The current value.
* @param bsonDocument The document with all field key-value pairs.
* @return Updated values to be used by SET operation.
*/
private static BsonValue getNewFieldValue(final BsonValue curValue,
final BsonDocument bsonDocument) {
if (
curValue != null && curValue.isString()
&& (((BsonString) curValue).getValue().contains(" + ")
|| ((BsonString) curValue).getValue().contains(" - "))
) {
String[] tokens = ((BsonString) curValue).getValue().split("\\s+");
boolean addNum = true;
// Pattern pattern = Pattern.compile(":?[a-zA-Z0-9]+");
Pattern pattern = Pattern.compile("[#:$]?[^\\s\\n]+");
Number newNum = null;
for (String token : tokens) {
if (token.equals("+")) {
addNum = true;
continue;
} else if (token.equals("-")) {
addNum = false;
continue;
}
Matcher matcher = pattern.matcher(token);
if (matcher.find()) {
String operand = matcher.group();
Number literalNum;
BsonValue topLevelValue = bsonDocument.get(operand);
BsonValue bsonValue = topLevelValue != null
? topLevelValue
: CommonComparisonExpressionUtils.getFieldFromDocument(operand, bsonDocument);

if (bsonValue == null && (literalNum = stringToNumber(operand)) != null) {
Number val = literalNum;
newNum =
newNum == null ? val : (addNum ? addNum(newNum, val) : subtractNum(newNum, val));
} else {
if (bsonValue == null) {
throw new IllegalArgumentException("Operand " + operand + " does not exist");
}
if (!bsonValue.isNumber() && !bsonValue.isDecimal128()) {
throw new IllegalArgumentException(
"Operand " + operand + " is not provided as number type");
}
Number val = getNumberFromBsonNumber((BsonNumber) bsonValue);
newNum =
newNum == null ? val : (addNum ? addNum(newNum, val) : subtractNum(newNum, val));
}
}
}
return getBsonNumberFromNumber(newNum);
} else if (curValue instanceof BsonDocument) {
if (curValue instanceof BsonDocument) {
BsonDocument doc = (BsonDocument) curValue;
if (doc.get("$IF_NOT_EXISTS") != null) {
return resolveIfNotExists(doc, bsonDocument);
Expand Down Expand Up @@ -823,34 +772,6 @@ private static String numberToString(Number number) {
throw new RuntimeException("Number type is not known for number: " + number);
}

/**
* Convert the given String to Number.
* @param number The String represented numeric value.
* @return The Number object.
*/
private static Number stringToNumber(String number) {
try {
return Integer.parseInt(number);
} catch (NumberFormatException e) {
// no-op
}
try {
return Long.parseLong(number);
} catch (NumberFormatException e) {
// no-op
}
try {
return Double.parseDouble(number);
} catch (NumberFormatException e) {
// no-op
}
try {
return NumberFormat.getInstance().parse(number);
} catch (ParseException e) {
return null;
}
}

/**
* Convert Number to BsonNumber.
* @param number The Number object.
Expand Down
4 changes: 2 additions & 2 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/Bson2IT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,9 @@ public void testUpdateExpressions() throws Exception {
*/

updateExp = "{\n" + " \"$SET\": {\n" + " \"Id1\": \"12345\",\n"
+ " \"NestedList1[0]\": \"NestedList1[0] + 12.22\",\n"
+ " \"NestedList1[0]\": { \"$ADD\": [ \"NestedList1[0]\", 12.22 ] },\n"
+ " \"NestedList1[3]\": null,\n" + " \"NestedList1[4]\": true,\n"
+ " \"attr_5[0]\": \"attr_5[0] - 10\"\n" + " }\n" + "}";
+ " \"attr_5[0]\": { \"$SUBTRACT\": [ \"attr_5[0]\", 10 ] }\n" + " }\n" + "}";

stmt = conn.prepareStatement("UPSERT INTO " + tableName
+ " VALUES (?,?) ON DUPLICATE KEY UPDATE COL = BSON_UPDATE_EXPRESSION(COL, '" + updateExp
Expand Down
20 changes: 15 additions & 5 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/Bson3IT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ public void testBsonOpsWithSqlConditionsUpdateSuccess() throws Exception {
.append("browserling", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 529.435")))
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonDouble(529.435))))))
.append("$UNSET",
new BsonDocument().append("track[0].shot[2][0].city.flame", new BsonNull()));

Expand Down Expand Up @@ -764,7 +766,9 @@ public void testBsonOpsWithDocumentConditionsUpdateSuccess() throws Exception {
.append("browserling", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 529.435")))
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonDouble(529.435))))))
.append("$UNSET",
new BsonDocument().append("track[0].shot[2][0].city.flame", new BsonNull()));

Expand Down Expand Up @@ -1163,7 +1167,9 @@ public void testBsonOpsWithSqlConditionsUpdateFailure() throws Exception {
.append("browserling", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 529.435")))
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonDouble(529.435))))))
.append("$UNSET",
new BsonDocument().append("track[0].shot[2][0].city.flame", new BsonNull()));

Expand Down Expand Up @@ -1536,7 +1542,9 @@ public void testBsonOpsWithDocumentConditionsUpdateFailure() throws Exception {
.append("browserling", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 529.435")))
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonDouble(529.435))))))
.append("$UNSET",
new BsonDocument().append("track[0].shot[2][0].city.flame", new BsonNull()));

Expand Down Expand Up @@ -1951,7 +1959,9 @@ public void testBsonOpsWithSqlConditionsUpdateSuccessWithTTL() throws Exception
.append("browserling", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 529.435")))
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonDouble(529.435))))))
.append("$UNSET",
new BsonDocument().append("track[0].shot[2][0].city.flame", new BsonNull()));

Expand Down
17 changes: 13 additions & 4 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/Bson4IT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.bson.BsonBinary;
import org.bson.BsonDocument;
import org.bson.BsonDouble;
import org.bson.BsonInt32;
import org.bson.BsonNull;
import org.bson.BsonString;
import org.bson.Document;
Expand Down Expand Up @@ -641,7 +642,9 @@ public void testConditionalUpsertReturnRow() throws Exception {
.append("browserling", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 529.435")))
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonDouble(529.435))))))
.append("$UNSET",
new BsonDocument().append("track[0].shot[2][0].city.flame", new BsonNull()));

Expand Down Expand Up @@ -725,7 +728,9 @@ public void testConditionalUpsertReturnRow() throws Exception {
.append("new_field1", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft_new_val"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 123")));
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonInt32(123))))));

stmt = conn.prepareStatement(
"UPSERT INTO " + tableName + " VALUES (?) ON DUPLICATE KEY UPDATE COL = CASE WHEN"
Expand Down Expand Up @@ -827,7 +832,9 @@ public void testConditionalUpsertReturnOldRow() throws Exception {
.append("browserling", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 529.435")))
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonDouble(529.435))))))
.append("$UNSET",
new BsonDocument().append("track[0].shot[2][0].city.flame", new BsonNull()));

Expand Down Expand Up @@ -907,7 +914,9 @@ public void testConditionalUpsertReturnOldRow() throws Exception {
.append("new_field1", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft_new_val"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 123")));
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonInt32(123))))));

stmt = conn.prepareStatement(
"UPSERT INTO " + tableName + " VALUES (?) ON DUPLICATE KEY UPDATE_ONLY COL = CASE WHEN"
Expand Down
8 changes: 6 additions & 2 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/Bson5IT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,9 @@ public void testBsonOpsWithSqlConditionsUpdateSuccess() throws Exception {
.append("browserling", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 529.435")))
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonDouble(529.435))))))
.append("$UNSET",
new BsonDocument().append("track[0].shot[2][0].city.flame", new BsonNull()));

Expand Down Expand Up @@ -487,7 +489,9 @@ public void testBsonOpsWithSqlConditionsUpdateFailure() throws Exception {
.append("browserling", new BsonBinary(PDouble.INSTANCE.toBytes(-505169340.54880095)))
.append("track[0].shot[2][0].city.standard[5]", new BsonString("soft"))
.append("track[0].shot[2][0].city.problem[2]",
new BsonString("track[0].shot[2][0].city.problem[2] + 529.435")))
new BsonDocument().append("$ADD", new BsonArray(Arrays.asList(
new BsonString("track[0].shot[2][0].city.problem[2]"),
new BsonDouble(529.435))))))
.append("$UNSET",
new BsonDocument().append("track[0].shot[2][0].city.flame", new BsonNull()));

Expand Down
Loading