bitflicker64 commented on code in PR #504: URL: https://github.com/apache/hugegraph-doc/pull/504#discussion_r4155693384
########## content/en/docs/clients/hugegraph-client.md: ########## @@ -119,6 +119,7 @@ The constraint information that PropertyKey allows to define includes: name, dat | asByte() | Byte | | asBlob() | Byte[] | | asDouble() | Double | +| asDecimal() | BigDecimal | Review 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. ########## content/en/docs/clients/restful-api/propertykey.md: ########## @@ -10,7 +10,7 @@ description: "PropertyKey REST API: Define data types and cardinality constraint Params Description: - 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 Review 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. -- 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]
