imbajin commented on code in PR #3209:
URL: https://github.com/apache/hugegraph/pull/3209#discussion_r4081391256
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/ConditionQuery.java:
##########
@@ -947,6 +947,11 @@ private static boolean numberEquals(Object number1, Object
number2) {
// Otherwise convert to BigDecimal to make two numbers comparable
Number n1 = NumericUtil.convertToNumber(number1);
Number n2 = NumericUtil.convertToNumber(number2);
+ if (n1 instanceof BigDecimal || n2 instanceof BigDecimal) {
Review Comment:
‼️ This exact-comparison change does not preserve DECIMAL conditions sent to
HStore. ConditionQuery.bytes() serializes BigDecimal as an untyped JSON number,
and QueryAdapter only writes a value class for Java primitive wrappers; the
Store's ConditionQuery.fromBytes() therefore reads the filter value as Double.
HStore's FilterIterator then compares against the BigDecimal property using
this rounded value, so equality and range queries for high-precision DECIMAL
values can miss or misclassify matches. Preserve BigDecimal type and precision
in the Store query wire format and its struct-side filter path.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/type/define/DataType.java:
##########
@@ -143,6 +156,65 @@ public <V> Number valueToNumber(V value) {
return number;
}
+ /**
+ * Convert a value to BigDecimal: BigDecimal as is, any other Number and a
+ * decimal string through their exact decimal representation. Float and
+ * Double go through Number.toString(), i.e. the shortest string that
+ * round-trips the binary value, so a client that already holds a lossy
+ * double gets that double, exactly.
+ *
+ * @return the BigDecimal, or null if the value is not a Number or String
+ * @throws IllegalArgumentException if the string is not a decimal number
+ */
+ /*
+ * Bounds for a DECIMAL value: at most DECIMAL_MAX_PRECISION significant
+ * digits and an absolute scale of at most DECIMAL_MAX_SCALE. uint256
+ * with 18 fraction digits is 96 digits, so both fit with room to spare,
+ * while "1E+999999999" (a few bytes on disk, a billion characters from
+ * toPlainString() on every read) is rejected before it is stored.
+ */
+ public static final int DECIMAL_MAX_PRECISION = 128;
+ public static final int DECIMAL_MAX_SCALE = 128;
+
+ public <V> BigDecimal valueToDecimal(V value) {
+ if (!this.isDecimal()) {
+ return null;
+ }
+ BigDecimal decimal;
+ if (value instanceof BigDecimal) {
+ decimal = (BigDecimal) value;
+ } else if (value instanceof BigInteger) {
+ decimal = new BigDecimal((BigInteger) value);
+ } else if (value instanceof Byte || value instanceof Short ||
+ value instanceof Integer || value instanceof Long) {
+ decimal = BigDecimal.valueOf(((Number) value).longValue());
+ } else if (!(value instanceof Number) && !(value instanceof String)) {
+ return null;
+ } else {
+ String text = value.toString().trim();
+ try {
+ decimal = new BigDecimal(text);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(String.format(
+ "Can't read '%s' as decimal", value));
+ }
+ }
+ return checkDecimalBounds(decimal);
+ }
+
+ public static BigDecimal checkDecimalBounds(BigDecimal decimal) {
+ int scale = Math.abs(decimal.scale());
Review Comment:
⚠️ This scale check is bypassed when decimal.scale() is Integer.MIN_VALUE
because Math.abs(Integer.MIN_VALUE) overflows and remains negative; a
precision-1 BigDecimal then passes the ±128 bound. PropertyKey deliberately
routes ready-made BigDecimal values through this check, and BytesBuffer
persists the scale, so a later toPlainString response can trigger an enormous
allocation. Compare scale directly with -DECIMAL_MAX_SCALE and
DECIMAL_MAX_SCALE in both the server and struct copies.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/io/HugeGraphSONModule.java:
##########
@@ -956,4 +961,56 @@ public Blob deserialize(JsonParser jsonParser,
return Blob.wrap(bytes);
}
}
+
+ private static class BigDecimalSerializer extends
StdSerializer<BigDecimal> {
+
+ public BigDecimalSerializer() {
+ super(BigDecimal.class);
+ }
+
+ @Override
+ public void serialize(BigDecimal decimal, JsonGenerator jsonGenerator,
+ SerializerProvider provider) throws IOException {
+ jsonGenerator.writeString(decimal.toPlainString());
Review Comment:
⚠️ The new GraphSON deserializer accepts BigDecimal values that bypass the
property bounds, while this serializer expands every BigDecimal with
toPlainString(). A Gremlin result such as new BigDecimal("1E+999999999") can
therefore request about a billion output characters and exhaust a response
worker's memory. Apply a safe bound to generic GraphSON BigDecimals or use an
exact string representation that does not expand the exponent.
--
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]
