SebastianGruza commented on code in PR #3209:
URL: https://github.com/apache/hugegraph/pull/3209#discussion_r4169121854
##########
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:
Right. `API.parseProperties()` now reads the filter by the same rule as a
request body, through the new `PropertiesDeserializer.parse(String)`: exact
fractions for the filter values and the members of a top-level array (a DECIMAL
filter keeps every digit), Jackson's number types inside an OBJECT value, so
the filter map equals the stored map. `fromJsonExact` remains only for
predicate operands in `TraversalUtil` (scalars and lists). Unit test
`testFilterParsesLikeABody`, and the REST regression
`EdgeApiTest.testObjectMapFilterOnAdjacency`: an edge with
`meta={ratio:0.25,n:3}`, the adjacency query with that filter finds it, with
`ratio:0.5` it returns an empty list.
##########
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:
Right, sorry for the red CI: this test was meant to go in round 6, since
`CacheManager.clearCache()` breaks the shared state of the suite (19 other
tests locally), and it stayed in the commit. Removed; the userdata reload from
storage is covered by the round-trip tests `testUserdataTypesSurviveAReload` in
`TextSerializerTest` and `BinarySerializerTest` (an exact DECIMAL default,
`rate: 0.85` still a Double, also after a re-write).
--
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]
