Copilot commented on code in PR #3035:
URL: https://github.com/apache/hugegraph/pull/3035#discussion_r3342379660
##########
hugegraph-struct/src/main/java/org/apache/hugegraph/struct/schema/PropertyKey.java:
##########
@@ -126,7 +126,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.validValueOrThrow(value);
+
+ // For SET cardinality, ensure we return a Set container and collapse
duplicates
+ if (this.cardinality == Cardinality.SET && normalized instanceof
Collection) {
+ return new LinkedHashSet<>((Collection<?>) normalized);
+ }
Review Comment:
`defaultValue()` currently normalizes first and then copies the normalized
`Collection` into a new `LinkedHashSet` for SET cardinality. This may
allocate/copy twice for JSON-deserialized defaults (`ArrayList`) and will also
copy even if the normalized value is already a `Set`. Pre-wrapping the raw
value to a `LinkedHashSet` before calling `validValueOrThrow()` avoids the
extra pass and lets normalization produce the correct container type directly.
##########
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.validValueOrThrow(value);
+
+ // For SET cardinality, ensure we return a Set container and collapse
duplicates
+ if (this.cardinality == Cardinality.SET && normalized instanceof
Collection) {
+ return new LinkedHashSet<>((Collection<?>) normalized);
+ }
Review Comment:
`defaultValue()` normalizes via `validValueOrThrow()` and then (for SET
cardinality) wraps the result into a new `LinkedHashSet`. For inputs coming
from JSON (typically `ArrayList`), this can do two passes/allocations (convert
to `ArrayList` in `convValue()`, then copy again into `LinkedHashSet`) and also
creates a fresh Set on every call even when the normalized value is already a
`Set`. Consider pre-wrapping the raw collection into a `LinkedHashSet` before
normalization so `validValueOrThrow()` can build the correct container type in
one step, and return it directly.
--
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]
