SebastianGruza commented on code in PR #3209:
URL: https://github.com/apache/hugegraph/pull/3209#discussion_r4154960885
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -222,14 +224,42 @@ protected static void checkUpdatingBody(Collection<?
extends Checkable> bodies)
}
@SuppressWarnings("unchecked")
+ /**
+ * Convert each plain filter value to the runtime type of its property
+ * key (a JSON fraction arrives as BigDecimal, an integer literal as
+ * Integer or Long): a DECIMAL value keeps every digit, a DOUBLE value
+ * becomes a double as before. Predicates (P.gt(...)), collections and
+ * values of unknown keys are left as they are.
+ */
+ protected static void normalizeProperties(HugeGraph g, Map<String, Object>
props) {
+ for (Map.Entry<String, Object> entry : props.entrySet()) {
+ Object value = entry.getValue();
+ if (value == null || value instanceof Collection || value
instanceof Map ||
+ value instanceof P) {
Review Comment:
Right. `TraversalUtil.predicateNumber`, `predicateArg` and `predicateArgs`
read through `JsonUtil.fromJsonExact`: a fraction in `P.eq(...)`,
`P.between(...)`, `P.within(...)` is a BigDecimal with every digit, and
`validPropertyValue` converts it to the property key's type when the condition
is built (a DOUBLE key still gets a double). `TraversalUtilTest`: a new case
with a 39-digit operand; the existing test now expects BigDecimal for
fractional literals, since that is the operand type now. `VertexApiTest`:
`P.eq(39 digits)` hits, `P.eq` with the last digit changed misses,
`P.within(near, exact)` hits, on rocksdb and hstore.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -222,14 +224,42 @@ protected static void checkUpdatingBody(Collection<?
extends Checkable> bodies)
}
@SuppressWarnings("unchecked")
+ /**
+ * Convert each plain filter value to the runtime type of its property
+ * key (a JSON fraction arrives as BigDecimal, an integer literal as
+ * Integer or Long): a DECIMAL value keeps every digit, a DOUBLE value
+ * becomes a double as before. Predicates (P.gt(...)), collections and
+ * values of unknown keys are left as they are.
+ */
+ protected static void normalizeProperties(HugeGraph g, Map<String, Object>
props) {
+ for (Map.Entry<String, Object> entry : props.entrySet()) {
+ Object value = entry.getValue();
+ if (value == null || value instanceof Collection || value
instanceof Map ||
Review Comment:
Right. `normalizeProperties` delegates to `TraversalUtil.validPropertyValue`
(now public), i.e. the rule the traversal itself uses: a list on a SINGLE key
converts every member, a scalar on a LIST/SET key stays a scalar for the
membership test, a list on a LIST key converts its members. `VertexApiTest`: a
DECIMAL LIST key `amounts`, a list filter `[39 digits, 1.0]` hits, a scalar
filter with the last digit changed misses.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/schema/UserdataDeserializer.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.schema;
+
+import java.io.IOException;
+import java.util.Map;
+
+import org.apache.hugegraph.api.graph.PropertiesDeserializer;
+import org.apache.hugegraph.schema.Userdata;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
+
+/**
+ * The userdata of a property key read like element properties: a JSON
+ * fraction becomes a BigDecimal with every digit, so a DECIMAL
+ * {@code ~default_value} reaches the property key exactly; everything else
+ * keeps Jackson's default types.
+ */
+public class UserdataDeserializer extends StdDeserializer<Userdata> {
+
+ private static final long serialVersionUID = 1L;
+
+ private static final PropertiesDeserializer PROPERTIES = new
PropertiesDeserializer();
+
+ public UserdataDeserializer() {
+ super(Userdata.class);
+ }
+
+ @Override
+ public Userdata deserialize(JsonParser parser, DeserializationContext
context)
+ throws IOException {
+ Map<String, Object> map = PROPERTIES.deserialize(parser, context);
Review Comment:
Right. The deserializer reads only `~default_value` exactly (through the now
public `PropertiesDeserializer.readValue`); every other entry goes through the
default `context.readValue(parser, Object.class)`, so `{"rate":0.85}` stays a
double and comes back as a JSON number. Tests: `PropertiesDeserializerTest`
(`rate` and a list in the metadata as doubles, the default as BigDecimal, also
when it is a list), `VertexApiTest` (`"rate":0.85` returned by GET as a number).
##########
hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/business/GraphStoreIterator.java:
##########
@@ -255,6 +256,10 @@ private <P extends BaseProperty<?>> List<Graphpb.Property>
buildProperties(
variant.setType(VariantType.VT_DOUBLE)
.setValueDouble((Double) v);
break;
+ case DECIMAL:
Review Comment:
Right for DECIMAL: the branch now encodes a collection as a JSON array of
plain strings (`["1.5","2"]`) in `VT_STRING`, a scalar as before. One remark
next to it: every other branch of this method casts the value to a scalar too
(`(Double) v`, `(Long) v`, ...), so a LIST/SET of any type ends in a
`ClassCastException` there independently of this PR; I am leaving that out of
scope here and can open a separate PR that encodes collections for every type,
if you want it.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/PropertyKey.java:
##########
@@ -116,7 +116,43 @@ 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));
+ }
+
+ /**
+ * The default value is kept in userdata in the runtime type of this
+ * key's data type (a JSON fraction arrives as BigDecimal: a DOUBLE key
+ * keeps a Double, a DECIMAL key the exact BigDecimal), 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.
+ */
+ @Override
+ public void userdata(String key, Object value) {
+ if (Userdata.DEFAULT_VALUE.equals(key)) {
+ value = this.normalizeDefaultValue(value);
Review Comment:
Right, and thanks for probing it on a real build: the round-4 override
converted `~default_value` for every type and on every schema read, so a DATE
`"2020-01-01"` came back as `"2020-01-01 00:00:00.000"` and an unconvertible
stored default broke loading the key. Done in the first of your forms: the
eager conversion applies to `DataType.DECIMAL` only; every other type keeps the
raw value as on master and `defaultValue()` converts it lazily when it is
applied; a DECIMAL default that does not convert stays raw instead of throwing
on the read (the error surfaces when the default is applied, as before). One
nuance the E2E caught: the API reads `~default_value` exactly, so for a
non-decimal key the BigDecimal is mapped back to the Double the parser produced
on master; a DOUBLE default `1.5` still comes back from GET as a number. Tests:
the new `PropertyKeyUserdataTest` in the unit suite (DECIMAL: BigDecimal,
string, list, `"not-a-number"` kept raw; DATE `"2020-01-01"` and `"not-a-date"
` kept raw; an INT LIST key with default `1` loads; a DOUBLE default and a
custom `rate` stay doubles) and `VertexApiTest`: a DATE key with
`"~default_value": "2020-01-01"` returns exactly that string from GET.
--
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]
