bitflicker64 commented on code in PR #771:
URL:
https://github.com/apache/hugegraph-toolchain/pull/771#discussion_r4078668028
##########
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/util/DataTypeUtil.java:
##########
@@ -200,7 +201,15 @@ private static Object parseSingleValue(String key, Object
rawValue,
}
} else if (dataType.isUUID()) {
return parseUUID(key, value);
+ } else if (dataType.isDecimal()) {
+ return parseDecimal(key, value);
} else if (dataType.isText()) {
+ if (value instanceof BigDecimal) {
+ // JSON fractions are parsed as BigDecimal (see JsonUtil);
+ // a TEXT key keeps the string the double path produced
+ // ("1.5", not "1.50"), so existing ids do not change
+ return Double.toString(((BigDecimal) value).doubleValue());
Review Comment:
⚠️ This branch runs for every `BigDecimal`, not only for the ones the JSON
parser now produces. JDBC sources are the common other producer: `JDBCFetcher`
and `RowFetcher` put `ResultSet.getObject(i)` straight into the line, and
MySQL/PostgreSQL `DECIMAL`/`NUMERIC` and every Oracle `NUMBER` column come back
as `BigDecimal`. Before this PR those values reached the `value instanceof
Number` check below and kept `value.toString()`.
Output for a TEXT key, old vs new (checked on a JDK):
| JDBC value | before | after |
|---|---|---|
| `12345` (Oracle `NUMBER(10)`) | `12345` | `12345.0` |
| `12.50` (`DECIMAL(10,2)`) | `12.50` | `12.5` |
| `12345678901234567890.12` | `12345678901234567890.12` |
`1.2345678901234567E19` |
So a JDBC column mapped to a TEXT key now stores different strings, the last
one lossy, and when that key is a primary key a reload creates new vertex ids
next to the old ones. This is the same silent id change the round-2 fix was
meant to prevent for JSON, moved to JDBC.
Please apply the `double` formatting only when the value came from the JSON
parser, for example when `source` is a `FileSource`/`HDFSSource` or
`KafkaSource` whose `format()` is `JSON`, and keep `value.toString()` for
everything else. A `DataTypeUtilTest` case that converts `new
BigDecimal("12.50")` for a TEXT key with a `JDBCSource` and expects `"12.50"`
would pin it.
--
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]
