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


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/Condition.java:
##########
@@ -280,6 +281,29 @@ public boolean test(Object first, Object second) {
             return this.tester.apply(first, second);
         }
 
+        /**
+         * Membership by value once a BigDecimal is involved: its equals() is
+         * scale sensitive (1.0 vs 1.00), while EQ compares decimals by value,
+         * so IN/NOT_IN do the same. Other numbers keep contains() semantics
+         * (an Integer 1 is not a Double 1.0 here, as before).
+         */
+        private static boolean collectionContains(Collection<?> values, Object 
value) {

Review Comment:
   ⚠️ The new numeric-by-value helper is wired only into IN/NOT_IN; 
RelationType.CONTAINS still calls Collection.contains (and the Struct copy does 
too). A DECIMAL LIST/SET value 1.00 therefore does not match a contains 
predicate for 1.0 because BigDecimal.equals is scale-sensitive, despite both 
values being numerically equal. Please use decimal value comparison for 
CONTAINS in both copies and add a differing-scale case.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/graph/PropertiesDeserializer.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hugegraph.api.graph;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonToken;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.JsonMappingException;
+
+/**
+ * Reads the "properties" object of a vertex or edge body so that a JSON
+ * fraction keeps every digit: it becomes a BigDecimal instead of a double,
+ * which is what a DECIMAL property key needs and what every numeric key
+ * narrows through DataType.valueToNumber as before. Only property values
+ * are read this way; the rest of the request body (job parameters, schema
+ * userdata, query options) keeps Jackson's default number types.
+ */
+public class PropertiesDeserializer extends JsonDeserializer<Map<String, 
Object>> {
+
+    @Override
+    public Map<String, Object> deserialize(JsonParser parser,
+                                           DeserializationContext ctxt)
+                                           throws IOException {
+        JsonToken token = parser.currentToken();
+        if (token == JsonToken.VALUE_NULL) {
+            return null;
+        }
+        if (token != JsonToken.START_OBJECT) {
+            throw JsonMappingException.from(parser,
+                  "Expected an object for 'properties', but got " + token);
+        }
+        return readObject(parser);
+    }
+
+    private static Map<String, Object> readObject(JsonParser parser)
+                                                  throws IOException {
+        Map<String, Object> object = new LinkedHashMap<>();
+        while (parser.nextToken() != JsonToken.END_OBJECT) {
+            String name = parser.currentName();
+            parser.nextToken();
+            object.put(name, readValue(parser));
+        }
+        return object;
+    }
+
+    private static List<Object> readArray(JsonParser parser)
+                                          throws IOException {
+        List<Object> array = new ArrayList<>();
+        while (parser.nextToken() != JsonToken.END_ARRAY) {
+            array.add(readValue(parser));
+        }
+        return array;
+    }
+
+    /** One value at the parser's current token: exact fractions, nested 
objects and arrays. */
+    public static Object readValue(JsonParser parser) throws IOException {
+        JsonToken token = parser.currentToken();
+        switch (token) {
+            case START_OBJECT:
+                return readObject(parser);
+            case START_ARRAY:
+                return readArray(parser);
+            case VALUE_STRING:
+                return parser.getText();
+            case VALUE_NUMBER_INT:
+                return parser.getNumberValue();
+            case VALUE_NUMBER_FLOAT:
+                // Exact: the literal's digits, not the nearest double
+                return parser.getDecimalValue();

Review Comment:
   ⚠️ This deserializer recursively turns every fractional token in properties 
into BigDecimal, including fractions nested inside a DataType.OBJECT value. The 
added BigDecimal serializer writes those values as JSON strings, so a 
non-DECIMAL object property containing a numeric ratio can round-trip with the 
ratio as a string. Please keep Jackson's prior numeric types for OBJECT 
contents while retaining exact parsing for DECIMAL values, and cover this 
request/response case.



##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/PropertyKey.java:
##########
@@ -116,7 +117,70 @@ public PropertyKey properties(Id... properties) {
 
     public void defineDefaultValue(Object value) {
         // TODO add a field default_value
-        this.userdata().put(Userdata.DEFAULT_VALUE, value);
+        this.userdata().put(Userdata.DEFAULT_VALUE, 
this.normalizeDefaultValue(value));
+    }
+
+    /**
+     * A DECIMAL key keeps its default value in userdata as the exact
+     * BigDecimal (a JSON fraction arrives as BigDecimal, a string is parsed),
+     * both when the key is defined through the API and when it is read back
+     * from the backend, so the value serializes the same way on every path.
+     * Every other data type keeps the raw value the user sent, as before:
+     * {@link #defaultValue()} converts it lazily when it is applied; only a
+     * BigDecimal (which exists solely because the API reads the default
+     * exactly) becomes the Double the default parser produced on master.
+     */
+    /** A BigDecimal from the exact parser as the Double the default parser 
gave. */
+    private static Object undoExact(Object value) {
+        if (value instanceof BigDecimal) {
+            return ((BigDecimal) value).doubleValue();
+        }
+        if (value instanceof Collection) {
+            List<Object> values = new ArrayList<>(((Collection<?>) 
value).size());
+            for (Object member : (Collection<?>) value) {
+                values.add(undoExact(member));
+            }
+            return value instanceof Set ? new LinkedHashSet<>(values) : values;
+        }
+        return value;
+    }
+
+    @Override
+    public void userdata(String key, Object value) {
+        if (Userdata.DEFAULT_VALUE.equals(key)) {
+            value = this.normalizeDefaultValue(value);
+        }
+        super.userdata(key, value);
+    }
+
+    @Override
+    public void userdata(Userdata userdata) {
+        E.checkArgumentNotNull(userdata, "userdata");
+        for (Map.Entry<String, Object> e : userdata.entrySet()) {
+            this.userdata(e.getKey(), e.getValue());
+        }
+    }
+
+    private Object normalizeDefaultValue(Object value) {
+        if (value == null) {
+            return value;
+        }
+        if (this.dataType != DataType.DECIMAL) {
+            return undoExact(value);
+        }
+        Object raw = value;
+        if (this.cardinality == Cardinality.SET && value instanceof Collection 
&&
+            !(value instanceof Set)) {
+            raw = new LinkedHashSet<>((Collection<?>) value);
+        }
+        try {
+            Object valid = this.validValue(raw);
+            return valid != null ? valid : value;
+        } catch (RuntimeException e) {

Review Comment:
   ⚠️ This fallback also runs when PropertyKeyBuilder.build() applies 
API-supplied userdata, so a malformed or out-of-bounds DECIMAL default is 
stored instead of rejected. PropertyKey.defaultValue() later calls 
validValueOrThrow(), so vertex creation fails when that property is omitted. 
Please keep tolerance only for legacy values loaded from storage and reject 
invalid DECIMAL defaults on create/append.



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