bitflicker64 commented on code in PR #771:
URL:
https://github.com/apache/hugegraph-toolchain/pull/771#discussion_r4155692368
##########
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/builder/ElementBuilder.java:
##########
@@ -269,7 +317,7 @@ protected Object mappingValue(String fieldName, Object
fieldValue) {
if (this.mapping().mappingValues().isEmpty()) {
return fieldValue;
}
- String fieldStrValue = String.valueOf(fieldValue);
+ String fieldStrValue = DataTypeUtil.mappingKey(fieldValue,
this.struct.input());
Review Comment:
Important: With any `value_mapping` in the struct, this line now sends a
DECIMAL column through `double`, and the column loses digits without an error.
`ElementMapping.mappingValue()` starts from `mappingValue = rawValue` and
replaces it only on a match. When nothing matches, the result is the lookup
string itself. `mappingKey()` now builds that string with
`Double.toString(value.doubleValue())`. Every property field goes through
`mappingValue()` (lines 539, 611, 702 and others), including fields that have
no mapping entry. So a struct that maps only `city` still turns `amount` into a
rounded string. `parseDecimal` then accepts that string as a valid decimal.
I checked this in jshell with Jackson 2.12.3 and
`USE_BIG_DECIMAL_FOR_FLOATS`:
| JSON literal | key from `mappingKey` | value at the DECIMAL key |
|---|---|---|
| `12345678901234567890.10` | `1.2345678901234567E19` |
`12345678901234567000` |
| `0.123456789012345678` | `0.12345678901234568` | `0.12345678901234568` |
Before round 5 this path used `String.valueOf`, which kept every digit of a
`BigDecimal`. `NullValuesTest.testJsonFractionMappingKey` checks the lookup key
only. No test sends a mapped struct into a DECIMAL key.
Requested change: use the double text only to look up the map. When no entry
matches, return the original `fieldValue`, not the lookup string. Please add a
test with a `value_mapping` on another field and a 20-digit JSON decimal loaded
into a DECIMAL key.
##########
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/util/DataTypeUtil.java:
##########
@@ -328,6 +353,59 @@ private static Number parseNumber(String key, Object value,
}
}
+ private static boolean fromJsonParser(InputSource source) {
+ if (source instanceof FileSource) {
+ return ((FileSource) source).format() == FileFormat.JSON;
+ }
+ if (source instanceof KafkaSource) {
+ return ((KafkaSource) source).getFormat() == FileFormat.JSON;
+ }
+ return false;
+ }
+
+ private static boolean withinDecimalBounds(BigDecimal decimal) {
+ try {
+ DataType.checkDecimalBounds(decimal);
+ return true;
+ } catch (IllegalArgumentException e) {
+ return false;
+ }
+ }
+
+ /**
+ * The string a JSON fraction had before it was read as BigDecimal: the
+ * shortest double representation ("1.5" for 1.50, "1.0E-7" for 1e-7).
+ * TEXT values and mapping lookups keep the keys they always had.
Review Comment:
Important: TEXT values and mapping keys now keep their old strings, but
vertex ids built from a JSON fraction still change.
`ElementBuilder.customizeId()` builds a CUSTOMIZE_STRING id with
`idValue.toString()` (line 354). `olapVertexId()` does the same for
CUSTOMIZE_STRING and PRIMARY_KEY (line 840). Edge endpoints go through
`customizeId()` too, because `EdgeBuilder` builds source and target through
`VertexKVPairs`. With no `value_mapping`, `idValue` is the raw `BigDecimal`
from `JsonValueDeser`, so it never passes through `jsonNumberText`.
| JSON id literal | id before this PR (Double) | id at this head
(BigDecimal) |
|---|---|---|
| `1.50` | `1.5` | `1.50` |
| `1e2` | `100.0` | `1E+2` |
| `1e-7` | `1.0E-7` | `1E-7` |
A reload of an existing JSON file creates new vertices next to the old ones,
and edges point at ids that no longer match. Integer literals are not affected
because they still parse as `Integer` or `Long`.
Requested change: format a JSON `BigDecimal` with `jsonNumberText` wherever
the loader turns a field value into a string id. That covers `customizeId()`
and `olapVertexId()`, and `splitField()` for flat ids. Please add a test that
loads a CUSTOMIZE_STRING id of `1.50` from a JSON row and expects `1.5`.
--
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]
