palashc opened a new pull request, #2496:
URL: https://github.com/apache/phoenix/pull/2496
### What changes were proposed in this pull request?
`UpdateExpressionUtils.getNewFieldValue` interpreted any `BsonString` SET
value
containing `" + "` or `" - "` as an arithmetic expression (splitting on
whitespace and resolving each token as a numeric field/literal). This change
removes that string-content heuristic: every plain string is now treated as a
literal. Arithmetic continues to be evaluated only from the explicit document
forms (`$ADD` / `$SUBTRACT` / `$IF_NOT_EXISTS`), which is what the
UpdateExpression parser already emits.
The now-unused `stringToNumber` helper and its imports (`Pattern`, `Matcher`,
`NumberFormat`, `ParseException`) were removed. `getNewFieldValue` has a
single
caller (`executeSetExpression`).
### Why are the changes needed?
The heuristic is fundamentally ambiguous: a literal value such as
`"Foo - Bar Baz Service"` is indistinguishable from an arithmetic expression
like `"fieldA - fieldB"`. Any DynamoDB `UpdateItem` (legacy
`AttributeUpdates`
PUT or `UpdateExpression` `SET attr = :val`) whose string value contained
`" + "` / `" - "` failed with:
```
java.lang.IllegalArgumentException: Operand ... does not exist at
org.apache.phoenix.expression.util.bson.UpdateExpressionUtils.getNewFieldValue(UpdateExpressionUtils.java:630)
```
Genuine arithmetic is unaffected because the parser encodes it as
`$ADD`/`$SUBTRACT` documents, not as raw strings.
### Does this PR introduce _any_ user-facing change?
Yes. Previously, a `SET` string value containing `" + "` or `" - "` was
(mis)interpreted as arithmetic — typically throwing `Operand ... does not
exist`. Now such values are stored verbatim as literals. This only affects
the
unreleased branch behavior of `BSON_UPDATE_EXPRESSION`; explicit
`$ADD`/`$SUBTRACT` arithmetic is unchanged.
### How was this patch tested?
- Updated `UpdateExpressionUtilsTest`:
- `testMixedSetExpressions` and `testUpdateExpression` now assert literal
storage for the former string-arithmetic cases.
- Added `testLiteralStringWithArithmeticOperatorsIsStoredVerbatim`.
- All 25 tests pass.
- End-to-end verified against the DynamoDB REST adapter (phoenix-adapters)
`UpdateItemIT` with new ITs covering literal strings containing `" - "`/`"
+ "`
via both legacy `AttributeUpdates` and `UpdateExpression :val` paths.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor (Claude Opus 4.8)
--
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]
