bitflicker64 commented on code in PR #3209:
URL: https://github.com/apache/hugegraph/pull/3209#discussion_r4185466203


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/PropertyKey.java:
##########
@@ -311,8 +394,20 @@ private <V, T> V convValue(V value) {
         if (value == null) {
             return null;
         }
-        if (this.checkValueType(value)) {
-            // Same as expected type, no conversion required
+        if (this.dataType() == DataType.OBJECT) {
+            // An OBJECT value is stored as it comes; a fraction the exact
+            // parser read as BigDecimal (a top-level scalar or a top-level
+            // array member of the request) goes back to the Double the
+            // default parser produced, so it serializes as a number
+            @SuppressWarnings("unchecked")
+            V plain = (V) undoExact(value);

Review Comment:
   Important: This branch runs for every OBJECT value, not just values that 
`PropertiesDeserializer` parsed exactly. Every write path goes through it 
(Gremlin `property()`, the Java API, REST), and so does every read from 
storage: `BinarySerializer.parseProperty()` calls `owner.addProperty()`, which 
builds the `HugeProperty`, and its constructor calls `validValueOrThrow()`. On 
master, `convValue()` returned an OBJECT value unchanged, because 
`Object.class.isInstance(value)` is always true. So a `BigDecimal` that an 
existing graph stored in an OBJECT property, for example from a Groovy literal 
like `property('meta', 0.1000000000000000000001)` (Groovy fractions are 
`BigDecimal`), now comes back as a `Double` after the upgrade and loses digits. 
I built hugegraph-core at 985b6c7 and checked this. 
`BytesBuffer.readProperty()` decodes the stored value as `BigDecimal 
0.1000000000000000000001`, then `validValueOrThrow()` returns `Double 0.1`. 
`[12345678901234567890.5]` becomes `[1.2345678901
 234567E19]`. `undoExact()` also rebuilds collections, so an OBJECT `TreeSet` 
comes back as a `LinkedHashSet`. The struct copy 
(`struct/schema/PropertyKey.java`, line 323) does the same on the Store side. 
Please keep `convValue()` returning OBJECT values untouched, as on master, and 
undo the exact parse only where it happened: in the API layer, once the 
property key is known. For example, `PropertiesDeserializer`'s caller could 
convert the `BigDecimal`s back for OBJECT keys before the properties reach 
`addVertex()`/`addEdge()`. Please also add a test that stores a `BigDecimal` in 
an OBJECT property through the core API and checks that it reads back unchanged.



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

Reply via email to