imbajin commented on code in PR #3209:
URL: https://github.com/apache/hugegraph/pull/3209#discussion_r4189081740
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/graph/BatchAPI.java:
##########
@@ -118,9 +133,15 @@ protected void updateExistElement(JsonElement oldElement,
JsonElement newElement
UpdateStrategy updateStrategy = kv.getValue();
if (oldElement.properties.get(key) != null &&
newElement.properties.get(key) != null) {
- Object value = updateStrategy.checkAndUpdateProperty(
- oldElement.properties.get(key),
- newElement.properties.get(key));
+ Object oldValue = oldElement.properties.get(key);
+ Object newValue = newElement.properties.get(key);
+ if (g != null) {
+ PropertyKey propertyKey = g.propertyKey(key);
+ oldValue = propertyKey.validValueOrThrow(oldValue);
+ newValue = propertyKey.validValueOrThrow(newValue);
+ }
Review Comment:
‼️ Blocking: yes. Summary: This normalizes DECIMAL set members and then
delegates to `INTERSECTION`/`ELIMINATE`, whose Java collection comparisons use
scale-sensitive `BigDecimal.equals`; `1.0` therefore fails to intersect or
remove `1.00`, silently producing the wrong update result. Please compare
DECIMAL members numerically in these strategies and cover differing scales.
Evidence: `BatchAPI.updateExistElement()` calls `validValueOrThrow()` before
the strategy; `UpdateStrategy` uses `Sets.intersection()` and
`Collection.removeAll()`.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -221,6 +224,32 @@ protected static void checkUpdatingBody(Collection<?
extends Checkable> bodies)
}
}
+ /**
+ * Convert each filter value to the runtime type of its property key the
+ * way the traversal does (TraversalUtil.validPropertyValue, which knows
+ * the key's cardinality: a list on a single key converts every member,
+ * a scalar on a LIST/SET key stays a scalar for membership). A JSON
+ * fraction arrives as BigDecimal, so a DECIMAL value keeps every digit
+ * and a DOUBLE value becomes a double as before. Predicates (P.gt(...))
+ * are converted when the traversal builds its conditions; values of
+ * unknown keys are left as they are.
+ */
+ protected static void normalizeProperties(HugeGraph g, Map<String, Object>
props) {
+ for (Map.Entry<String, Object> entry : props.entrySet()) {
+ Object value = entry.getValue();
+ if (value == null || value instanceof P) {
+ continue;
+ }
+ PropertyKey pkey;
+ try {
+ pkey = g.propertyKey(entry.getKey());
+ } catch (NotFoundException e) {
+ continue;
+ }
+ entry.setValue(TraversalUtil.validPropertyValue(value, pkey));
Review Comment:
⚠️ Blocking: no. Summary: A JSON array filter for a DECIMAL SET remains a
`List<BigDecimal>`, while stored values are `Set<BigDecimal>`, so the equality
predicate never matches the same members. Please normalize filter arrays to the
key's collection semantics or use a set-aware predicate, and cover a matching
DECIMAL SET list-filter case. Evidence: `PropertiesDeserializer` returns arrays
as lists; `TraversalUtil.validPropertyValue()` and `PropertyKey.convValue()`
preserve the container type, and `Condition` equality compares collections as
values.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
