SebastianGruza commented on code in PR #771:
URL:
https://github.com/apache/hugegraph-toolchain/pull/771#discussion_r4066840223
##########
hugegraph-client/src/main/java/org/apache/hugegraph/client/RestClient.java:
##########
@@ -49,6 +52,11 @@ public class RestClient extends AbstractRestClient {
SimpleModule module = new SimpleModule();
module.addDeserializer(Path.class, new PathDeserializer());
RestResult.registerModule(module);
+
+ // Request bodies: a decimal goes as a plain string, see JsonUtil
+ SimpleModule decimals = new SimpleModule();
+ decimals.addSerializer(BigDecimal.class, new BigDecimalSerializer());
+ JsonUtilCommon.registerModule(decimals);
Review Comment:
Good catch, thanks: a client feeding JDBC `NUMERIC` into an `asDouble()` key
would have got a 400. I changed the approach in a57545f3: a `BigDecimal` is no
longer a string but a plain JSON number in `toPlainString()` form (`1000`, not
`1E+3`; every digit). For numeric keys nothing changes against any server,
since `valueToNumber` accepts any `Number`; for DECIMAL the exactness comes
from the server side: in apache/hugegraph#3209 (0146849b) Jersey now reads JSON
fractions as `BigDecimal` (`ObjectMapperResolver`,
`USE_BIG_DECIMAL_FOR_FLOATS`), so a 39-digit literal reaches a DECIMAL key
intact while a DOUBLE key narrows it to a double as before.
`VertexApiTest.testCreateWithBigDecimalOnDoubleKey` writes `new
BigDecimal("1.5")` to a DOUBLE key and reads 1.5 back; it runs in the regular
`ApiTestSuite`, i.e. against the 1.7.0 server in CI as well. A Gremlin binding
holding a `BigDecimal` stays a number. There is no public-behaviour change left
to note in the release notes; the PR descr
iption's "public API" section is updated accordingly.
##########
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/util/DataTypeUtil.java:
##########
@@ -328,6 +331,21 @@ private static Number parseNumber(String key, Object value,
}
}
+ private static BigDecimal parseDecimal(String key, Object rawValue) {
+ BigDecimal decimal;
+ try {
+ decimal = DataType.DECIMAL.valueToDecimal(rawValue);
Review Comment:
Done the way you suggest: the loader's `JsonUtil.MAPPER` has
`USE_BIG_DECIMAL_FOR_FLOATS`, so `JsonLineParser` hands fractions over as
`BigDecimal` and `parseNumber` narrows them for DOUBLE/FLOAT/INT/LONG keys as
before (integer literals still arrive as Integer/Long). Lists:
`parseMultiValues` now converts collection elements one by one through
`parseSingleValue` when not all of them already have the right type, so `[1.10,
2, 3E-18]` yields three `BigDecimal`s instead of "must be String type"; my
earlier test asserted that failure and is corrected.
`DataTypeUtilTest.testConvertDecimalFromJsonLine` parses a line with a 39-digit
value, a mixed list and a DOUBLE key. I kept accepting `Double` input for
DECIMAL keys (shortest round-trip representation, the same rule as the server)
rather than rejecting it: after this change a `Double` no longer comes out of
the JSON parser at all.
##########
hugegraph-client/src/main/java/org/apache/hugegraph/serializer/direct/util/BytesBuffer.java:
##########
@@ -750,6 +751,13 @@ public void writeProperty(DataType dataType, Object value)
{
this.writeLong(uuid.getMostSignificantBits());
this.writeLong(uuid.getLeastSignificantBits());
break;
+ case DECIMAL:
+ // Same layout as the server: unscaled two's-complement
+ // bytes followed by the scale, exact for any precision
+ BigDecimal decimal = dataType.valueToDecimal(value);
Review Comment:
Done: both `valueToDecimal` copies end in `checkDecimalBounds` with the
#3209 constants (`DECIMAL_MAX_PRECISION = 128`, `DECIMAL_MAX_SCALE = 128`), so
`BytesBuffer.writeProperty` rejects `1E+999999999` before anything reaches
HBase. `DecimalDataTypeTest.testDecimalBounds` covers 128 digits and `1E±128`
as the boundary, 129 digits, `1E-129` and `1E+999999999` on both copies and on
`writeProperty`.
--
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]
