imbajin commented on code in PR #3209:
URL: https://github.com/apache/hugegraph/pull/3209#discussion_r4168654813
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -229,7 +258,8 @@ protected static Map<String, Object> parseProperties(String
properties) {
Map<String, Object> props = null;
try {
- props = JsonUtil.fromJson(properties, Map.class);
+ // Exact fractions: a DECIMAL filter keeps every digit
+ props = JsonUtil.fromJsonExact(properties, Map.class);
Review Comment:
⚠️ Important. Blocking: yes. Summary: fromJsonExact parses fractions
recursively as BigDecimal, including nested maps in query filters. The
request-body parser deliberately leaves the same OBJECT-map numbers as Double,
while normalizeProperties keeps OBJECT values unchanged; map equality therefore
treats an otherwise identical filter as different and edge adjacency queries
omit matches. Please preserve Jackson's prior number types for OBJECT filters
or normalize nested values by schema while retaining DECIMAL precision, and add
a REST adjacency regression for meta={ratio:0.25}. Evidence: this filter value
reaches EdgeAPI.list and is passed to traversal.has after normalization.
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/PropertyKeyCoreTest.java:
##########
@@ -741,4 +777,95 @@ public void
testDuplicatePropertyKeyWithDifferentProperties() {
.create();
});
}
+
+ @Test
+ public void testAddPropertyKeyWithDecimalType() {
+ SchemaManager schema = graph().schema();
+ PropertyKey balance = schema.propertyKey("balance")
+ .asDecimal()
+ .valueSingle()
+ .create();
+
+ Assert.assertEquals("balance", balance.name());
+ Assert.assertEquals(DataType.DECIMAL, balance.dataType());
+ Assert.assertEquals(Cardinality.SINGLE, balance.cardinality());
+ Assert.assertEquals(DataType.DECIMAL,
+ graph().propertyKey("balance").dataType());
+
+ // values are normalised to BigDecimal, exactly
+ String uint256Max = "115792089237316195423570985008687907853" +
+ "269984665640564039457584007913129639935";
+ Assert.assertEquals(new BigDecimal(uint256Max),
+ balance.validValue(uint256Max));
+ Assert.assertEquals(new BigDecimal("42"), balance.validValue(42L));
+ Assert.assertEquals(new BigDecimal("0.1"), balance.validValue(0.1D));
+ Assert.assertNull(balance.validValue(true));
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ balance.validValue("1,5");
+ }, e -> {
+ Assert.assertContains("Can't read '1,5' as decimal",
+ e.getMessage());
+ });
+ // bounds hold for strings and for ready-made BigDecimals alike
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ balance.validValue("1E+999999999");
+ }, e -> {
+ Assert.assertContains("out of bounds", e.getMessage());
+ });
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ balance.validValue(new BigDecimal("1E+999999999"));
+ }, e -> {
+ Assert.assertContains("out of bounds", e.getMessage());
+ });
+ Assert.assertEquals(new BigDecimal("1E+128"),
+ balance.validValue(new BigDecimal("1E+128")));
+
+ // SUM/MAX/MIN aggregate types are allowed like on any numeric key
+ PropertyKey total = schema.propertyKey("total")
+ .asDecimal()
+ .calcSum()
+ .create();
+ Assert.assertEquals(AggregateType.SUM, total.aggregateType());
+
+ // decimal lists and sets
+ PropertyKey amounts = schema.propertyKey("amounts")
+ .asDecimal()
+ .valueList()
+ .create();
+ Assert.assertEquals(Cardinality.LIST, amounts.cardinality());
+ Assert.assertEquals(ImmutableList.of(new BigDecimal("1"),
+ new BigDecimal("2.5")),
+ amounts.validValue(ImmutableList.of("1", "2.5")));
+ }
+
+ /**
+ * Userdata read back from the backend keeps its types: a fractional
+ * custom entry stays a Double and a DECIMAL default stays the exact
+ * BigDecimal, on every schema element.
+ */
+ @Test
+ public void testUserdataTypesSurviveABackendReload() {
+ SchemaManager schema = graph().schema();
+ schema.propertyKey("fee").asDecimal()
+ .userdata(Userdata.DEFAULT_VALUE, new
BigDecimal("0.1234567890123456789"))
+ .userdata("rate", 0.85d)
+ .create();
+ schema.propertyKey("name").asText().create();
+ schema.vertexLabel("account").properties("name",
"fee").primaryKeys("name")
+ .nullableKeys("fee").userdata("rate", 0.85d).create();
+
+
org.apache.hugegraph.backend.cache.CacheManager.instance().clearCache();
Review Comment:
⚠️ Important. Blocking: yes. Summary: This global cache clear follows
creation of the shared CoreTestSuite fee property key and account vertex label,
but the test leaves that schema behind. Current core CI fails repeatedly in
BaseCoreTest.clearSchema because fee is still referenced by account (817
errors); the HStore core job reports the same exception. Please clean up the
created schema in this test or avoid clearing shared cache state. Evidence:
https://github.com/apache/hugegraph/actions/runs/37003645045/job/110840203173
and
https://github.com/apache/hugegraph/actions/runs/37003644747/job/110837746563.
--
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]
