Repository navigation
docs: add the DECIMAL property data type - #504
SebastianGruza wants to merge 4 commits into
Conversation
…he/hugegraph-toolchain#771) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The new REST contract says DECIMAL responses are JSON numbers, but the paired server head serializes BigDecimal responses as strings, so clients following this text may decode them incorrectly. Evidence: apache/hugegraph at 0bb7724a406d9ee49ab5bfee719aa440ee6ab002 registers a BigDecimal string serializer in JsonUtil; JsonUtilTest confirms quoted output.
|
|
||
| - name: The name of the property type, required. | ||
| - data_type: The data type of the property type, including: bool, byte, int, long, float, double, text, blob, date, uuid. The default data type is `text` (Represent a `string` type) | ||
| - data_type: The data type of the property type, including: bool, byte, int, long, float, double, decimal, text, blob, date, uuid. The default data type is `text` (Represent a `string` type). `decimal` holds an exact decimal number (Java `BigDecimal`): at most 128 significant digits and a scale of at most 128 in either direction; it is sent and returned as a plain JSON number with every digit, so a client must not parse it as a double |
There was a problem hiding this comment.
There was a problem hiding this comment.
Right. Now: in a request a JSON number or a string, both read exactly; in a response a string (example "amount": "12345678901234567890.123456789012345678"), which a client parses with new BigDecimal(String), never as a double. The Chinese paragraph says the same.
…tring Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Updated in 4e4d3f1: DECIMAL is accepted as a JSON number or a string and returned as a string, in both languages; matches apache/hugegraph#3209 at d7eb80c8. |
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The DECIMAL text now matches the paired server head: bounds of 128 digits and scale within plus or minus 128, exact JSON number or string input, string output. Two gaps remain: these pages document current master, which has no DECIMAL type or asDecimal() yet, and the new text does not say that a decimal key cannot be indexed, used as a primary key or sort key, or given an OLAP index write type. Evidence: apache/hugegraph#3209 at d7eb80c8 (DataType.DECIMAL_MAX_PRECISION/SCALE and checkDecimalBounds, PropertiesDeserializer.readValue, HugeGraphSONModule.BigDecimalSerializer registered into JsonUtil through registerCommonSerializers, VertexApiTest string assertions, the isDecimal() checks in VertexLabelBuilder, EdgeLabelBuilder, IndexLabelBuilder and PropertyKeyBuilder); apache/hugegraph-toolchain#771 at 991a97f7 adds PropertyKey.Builder.asDecimal(); master DataType.java and client PropertyKey.java contain no DECIMAL or asDecimal; restful-api/_index.md says the section documents current master; no CI checks are reported on this head.
| | asByte() | Byte | | ||
| | asBlob() | Byte[] | | ||
| | asDouble() | Double | | ||
| | asDecimal() | BigDecimal | |
There was a problem hiding this comment.
Important: Merging this before apache/hugegraph#3209 and apache/hugegraph-toolchain#771 documents a type that master does not have. restful-api/_index.md says this section documents current master, and this client guide follows master too. Today DataType.java on hugegraph master has no DECIMAL, and PropertyKey.Builder on toolchain master has no asDecimal(). A reader who sends "data_type": "DECIMAL" or calls asDecimal() gets an error. Both code PRs are still open (server head d7eb80c8, toolchain head 991a97f7). Please hold this PR until both are merged, or mark the type with the release it ships in. This applies to the matching lines in content/cn too.
There was a problem hiding this comment.
Agreed, this PR waits for the merge of apache/hugegraph#3209 and apache/hugegraph-toolchain#771. Both pages (en/cn) now carry a release note: the DECIMAL type ships with those PRs, and until they are merged sending "data_type": "DECIMAL" or calling asDecimal() fails. Marking the PR as on hold; I will lift it after the merges.
|
|
||
| - name: The name of the property type, required. | ||
| - data_type: The data type of the property type, including: bool, byte, int, long, float, double, text, blob, date, uuid. The default data type is `text` (Represent a `string` type) | ||
| - data_type: The data type of the property type, including: bool, byte, int, long, float, double, decimal, text, blob, date, uuid. The default data type is `text` (Represent a `string` type). `decimal` holds an exact decimal number (Java `BigDecimal`): at most 128 significant digits and a scale of at most 128 in either direction; in a request it may be sent as a JSON number or as a string and is read exactly either way; in a response it is returned as a string (e.g. `"amount": "12345678901234567890.123456789012345678"`), so a client parses it with `new BigDecimal(String)`, never as a double |
There was a problem hiding this comment.
Minor: The new decimal text does not list the limits that #3209 enforces. At d7eb80c8 the server rejects a decimal key as an index field of any type (IndexLabelBuilder), as a vertex primary key (VertexLabelBuilder), as an edge sort key (EdgeLabelBuilder), and with write type OLAP_SECONDARY or OLAP_RANGE (PropertyKeyBuilder, only OLAP_COMMON is allowed). The errors are clear, but a user designing a schema from this page finds out only at creation time. Could you add one sentence here and in the Chinese page, for example: a decimal key cannot be indexed, used as a primary key or sort key, or set to an OLAP index write type? The writeType table in hugegraph-client.md could carry the same note.
There was a problem hiding this comment.
Done: one sentence on the limits in propertykey.md (en/cn) and next to the writeType table in hugegraph-client.md: a DECIMAL key cannot be an index field, a primary key or a sort key, and the only allowed write type is OLAP_COMMON.
…te type) and the release it ships in Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
2c8bcbe: the DECIMAL limits (index, primary key, sort key, write type) in both languages, plus a release note. On hold until apache/hugegraph#3209 and apache/hugegraph-toolchain#771 are merged. |
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The client row leaves #771 in the wrong repository context, and the REST paragraph makes server support appear to depend on toolchain release. Evidence: apache/hugegraph#3209 adds DataType.DECIMAL; apache/hugegraph-toolchain#771 adds Builder.asDecimal().
| | asByte() | Byte | | ||
| | asBlob() | Byte[] | | ||
| | asDouble() | Double | | ||
| | asDecimal() | BigDecimal (exact decimal, not indexable, not a primary/sort key; from the release with apache/hugegraph#3209 and #771) | |
There was a problem hiding this comment.
🧹 Minor. Blocking: no. Summary: Please qualify #771 as apache/hugegraph-toolchain#771 in this row and the Chinese counterpart. Evidence: This row also names apache/hugegraph#3209, but an unqualified #771 resolves within apache/hugegraph-doc; the asDecimal() builder API is introduced by apache/hugegraph-toolchain#771.
There was a problem hiding this comment.
Done: apache/hugegraph-toolchain#771 in the English and Chinese rows.
|
|
||
| - name: The name of the property type, required. | ||
| - data_type: The data type of the property type, including: bool, byte, int, long, float, double, text, blob, date, uuid. The default data type is `text` (Represent a `string` type) | ||
| - data_type: The data type of the property type, including: bool, byte, int, long, float, double, decimal, text, blob, date, uuid. The default data type is `text` (Represent a `string` type). `decimal` holds an exact decimal number (Java `BigDecimal`): at most 128 significant digits and a scale of at most 128 in either direction; in a request it may be sent as a JSON number or as a string and is read exactly either way; in a response it is returned as a string (e.g. `"amount": "12345678901234567890.123456789012345678"`), so a client parses it with `new BigDecimal(String)`, never as a double. A decimal key cannot be indexed, used as a primary key or an edge sort key, or given an OLAP index write type (only `OLAP_COMMON`). Available from the release that ships apache/hugegraph#3209 and apache/hugegraph-toolchain#771 |
There was a problem hiding this comment.
There was a problem hiding this comment.
Done: the REST page scopes decimal to the HugeGraph Server release that ships apache/hugegraph#3209, and names the hugegraph-client release with apache/hugegraph-toolchain#771 only for the asDecimal() builder; English and Chinese.
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The DECIMAL text matches apache/hugegraph#3209 at its current head 2034d240 (bounds, number or string input, string output, index, primary key and sort key limits). One gap remains: the Java client page does not carry the write type limit that the reply on the earlier thread says was added. Evidence: PropertyKeyBuilder.checkOlap() at 2034d240 rejects any write type other than OLAP_COMMON for a decimal OLAP key; content/en/docs/clients/hugegraph-client.md at 2c8bcbe changes only the asDecimal() row, and the writeType table is unchanged; apache/hugegraph-toolchain#771 at 5255714c adds PropertyKey.Builder.asDecimal().
| | asByte() | Byte | | ||
| | asBlob() | Byte[] | | ||
| | asDouble() | Double | | ||
| | asDecimal() | BigDecimal (exact decimal, not indexable, not a primary/sort key; from the release with apache/hugegraph#3209 and #771) | |
There was a problem hiding this comment.
Minor: The reply on the earlier limits thread says the write type limit was added next to the writeType table here, but at 2c8bcbe this page only changes this row, and the row lists "not indexable, not a primary/sort key" without the write type. On apache/hugegraph#3209 at 2034d240, PropertyKeyBuilder.checkOlap() throws NotAllowException for a decimal key with OLAP_SECONDARY or OLAP_RANGE, so asDecimal().writeType(WriteType.OLAP_RANGE) fails at create time and this page does not warn about it. Please add the limit to this row or under the writeType table (for example: a decimal key allows only OLTP or OLAP_COMMON), and make the same change in content/cn/docs/clients/hugegraph-client.md line 123.
There was a problem hiding this comment.
Right, my round-6 reply claimed a change that was not in the commit. The asDecimal() row and a sentence under the writeType table now carry it: a decimal key allows only OLTP or OLAP_COMMON, the server rejects OLAP_SECONDARY and OLAP_RANGE at create time; English line 122 and Chinese line 123.
…e qualified, REST availability scoped to the server release
|
da2ed53: the write type limit on the client page, the toolchain reference qualified, REST availability scoped to the server release. Still on hold until apache/hugegraph#3209 and apache/hugegraph-toolchain#771 are merged. |
|
Closing together with apache/hugegraph#3209 and apache/hugegraph-toolchain#771: the project sees no demand for a native DECIMAL type for now (see the discussion on #3209). Thanks for the reviews. |
3 participants
Purpose of the PR
Docs for the DECIMAL property data type: apache/hugegraph#3209 (server) and apache/hugegraph-toolchain#771 (client, loader, hubble, spark). Paired docs change requested in the review of #3209.
Main Changes
restful-api/propertykey.md(en, cn):decimalin thedata_typelist, with its bounds (at most 128 significant digits, scale within ±128) and the wire form (a plain JSON number with every digit, so a client must not parse it as a double).clients/hugegraph-client.md(en, cn):asDecimal() | BigDecimalin the data type table.Verifying these changes
Markdown only; the wording matches the constants and behaviour in the two PRs (
DataType.DECIMAL_MAX_PRECISION/SCALE,PropertiesDeserializer,BigDecimalSerializer).🤖 Generated with Claude Code