bitflicker64 commented on code in PR #3209:
URL: https://github.com/apache/hugegraph/pull/3209#discussion_r4156707420
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/serializer/BinarySerializer.java:
##########
@@ -1237,8 +1237,8 @@ private void readUserdata(SchemaElement schema) {
byte[] userdataBytes = column(HugeKeys.USER_DATA);
String userdataStr = StringEncoding.decode(userdataBytes);
@SuppressWarnings("unchecked")
- Map<String, Object> userdata = JsonUtil.fromJson(userdataStr,
- Map.class);
+ Map<String, Object> userdata = JsonUtil.fromJsonExact(userdataStr,
Review Comment:
Important: Reading schema userdata with `fromJsonExact` changes custom
metadata on every schema element after a restart, not only the DECIMAL default.
`readUserdata()` here, and the same method in `TextSerializer` at line 914,
is used for vertex labels, edge labels, index labels and property keys. With
`USE_BIG_DECIMAL_FOR_FLOATS` every stored fraction comes back as a
`BigDecimal`. `PropertyKey.userdata(String, Object)` turns only
`~default_value` back into a `Double`; every other key, and every key on the
other schema types, goes to `SchemaElement.userdata()` unchanged. `JsonUtil`
then writes that `BigDecimal` as a string through the serializer added to
`registerCommonSerializers()`.
So `schema.vertexLabel("person").userdata("rate", 0.85)` returns `"rate":
0.85` from `GET /schema/vertexlabels/person` until the server restarts (or
another node loads the schema from the backend), and `"rate": "0.85"` after.
The next schema write (for example `append` of a property) stores the string,
so the type change becomes permanent. `UserdataDeserializer` was written to
keep `{"rate":0.85}` a number on the API path, and `PropertyKeyUserdataTest`
checks that only on an in-memory key, never through a backend round trip.
I checked the parse and write steps with the gremlin-shaded 3.5.1 Jackson
and a `BigDecimal` serializer that writes `toPlainString()`, as the PR does:
the stored `{"rate":0.85}` reads as `Double` and writes `{"rate":0.85}` with
`fromJson`, and reads as `BigDecimal` and writes `{"rate":"0.85"}` with the
exact reader.
The exact reader is not needed here. A DECIMAL `~default_value` is held as a
`BigDecimal` and `writeUserdata()` stores it as a JSON string, so plain
`fromJson` reads a `String` and `normalizeDefaultValue()` converts it back
exactly.
Requested change: go back to `JsonUtil.fromJson(userdataStr, Map.class)` in
both `readUserdata()` methods. Add a core test that sets a fractional custom
userdata entry on a vertex label and a DECIMAL default on a property key,
reloads the schema from the backend, and checks that the first is still a
`Double` and the second is still the exact `BigDecimal`.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -222,14 +225,41 @@ protected static void checkUpdatingBody(Collection<?
extends Checkable> bodies)
}
@SuppressWarnings("unchecked")
Review Comment:
Minor: The new method was inserted between `@SuppressWarnings("unchecked")`
and `parseProperties()`, so the annotation now applies to
`normalizeProperties()` and `parseProperties()` loses it. The annotation also
sits above the Javadoc, so the Javadoc tool no longer attaches that comment to
`normalizeProperties()`. `PropertyKey.java` has the same problem at lines
123-133: the block that describes `normalizeDefaultValue()` sits directly above
the separate Javadoc of `undoExact()`, so neither method gets the right
documentation.
Requested change: put `@SuppressWarnings("unchecked")` back directly above
`parseProperties()`, and move the `normalizeDefaultValue()` Javadoc in
`PropertyKey` to that method.
--
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]
