SebastianGruza commented on code in PR #3209:
URL: https://github.com/apache/hugegraph/pull/3209#discussion_r4033455255
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/type/define/DataType.java:
##########
@@ -43,7 +45,14 @@ public enum DataType implements SerialEnum {
TEXT(8, "text", String.class),
BLOB(9, "blob", Blob.class),
DATE(10, "date", Date.class),
- UUID(11, "uuid", UUID.class);
+ UUID(11, "uuid", UUID.class),
+ /*
+ * Arbitrary-precision decimal (java.math.BigDecimal). Stored exactly; not
a
+ * "number" in the isNumber() sense because it has no fixed-width, sortable
+ * encoding, so it can't be a sort key, a range/secondary index field or an
+ * OLAP range property.
+ */
+ DECIMAL(12, "decimal", BigDecimal.class);
Review Comment:
Done in 834ef89, thanks for tracing that path. I reject a decimal as a
primary key rather than adding a canonical encoding: it is consistent with this
PR's rule (no sort key, no index, no OLAP write type with an index), and a
lossless id would also have to settle scale normalisation (are `1.0` and `1`
the same key or different ones?) and touch `SplicingIdGenerator`, which I would
rather not do in this PR. The guard sits in
`VertexLabelBuilder.checkPrimaryKeys()`, next to the existing check that the
key belongs to the properties, in the same shape as the decimal sort-key
rejection in `EdgeLabelBuilder`: `The primary key 'balance' of vertex label
'account' can't be a decimal property`. It covers a composite key that has a
decimal among its fields too. The REST batch path you mention (hashing the raw
string before core normalisation) no longer matters, since such a label cannot
exist.
Test: `VertexLabelCoreTest.testAddVertexLabelWithDecimalPrimaryKey`: a
single decimal primary key and a composite `(name, balance)` key are rejected
with that message, the label does not exist afterwards, and a decimal as a
plain property next to a text primary key passes. `VertexLabelCoreTest` 53/53
on rocksdb, 123/123 together with `PropertyKeyCoreTest` and
`EdgeLabelCoreTest`. End to end: a new R2 check in `cluster/decimal_e2e.py`
(`POST /schema/vertexlabels` with `primary_keys: [balance]` → 400 `The primary
key 'd2g_balance' of vertex label 'd2g_badpk' can't be a decimal property`); on
rocksdb with a dist from this head 38 PASS, 0 FAIL, 4 N-A, log
`results/decimal/e2e/after3-rocksdb.log` in
https://github.com/SebastianGruza/hugegraph-validation. The limits table in the
PR description and in the docs now lists the primary key as well.
--
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]
