dpol1 commented on code in PR #3035:
URL: https://github.com/apache/hugegraph/pull/3035#discussion_r3341561792
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/PropertyKey.java:
##########
@@ -121,7 +121,23 @@ public void defineDefaultValue(Object value) {
public Object defaultValue() {
// TODO add a field default_value
- return this.userdata().get(Userdata.DEFAULT_VALUE);
+ Object value = this.userdata().get(Userdata.DEFAULT_VALUE);
+ if (value == null) {
+ return null;
+ }
+
+ // Userdata is reloaded from JSON as a raw Map, so a typed default
+ // value (e.g. Date) comes back as a String. Normalize it to the
+ // runtime type expected by this property key's data type. Idempotent
+ // for values already of the expected type.
+ Object normalized = this.validValue(value);
+
+ // For SET cardinality, ensure we return a Set container and collapse
duplicates
Review Comment:
Hey @imbajin, pushed the missing server tests (`VertexCoreTest` +
serializers). I used duplicate `String` arrays to properly mimic the JSON
reload and verify the `Set` collapse.
About Copilot's comments: I switched to `validValueOrThrow()` (it was
reasonable), but ignored the tip to change `convValue()`. Forcing a `Set` there
breaks the `<V> V` contract and throws `ClassCastException`s in existing tests.
Keeping the fix inside `defaultValue()` is much safer.
Good to merge?
--
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]
