HyukjinKwon commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4171563720
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,93 @@ def handle_array(value_pos_list: List[int]) -> str:
if isinstance(value, bytes):
# decoding simply converts byte array to string
return '"' + base64.b64encode(value).decode("utf-8") + '"'
- if isinstance(value, (datetime.date, datetime.datetime)):
+ if isinstance(value, float):
+ return cls._float_to_json(value, is_float32)
+ if isinstance(value, decimal.Decimal):
+ return cls._decimal_to_json(value)
+ if isinstance(value, datetime.datetime):
+ return '"' + cls._timestamp_to_json(value) + '"'
+ if isinstance(value, datetime.date):
return '"' + str(value) + '"'
return str(value)
+ # The helpers below format scalars the same way as the JVM
`Variant.toJson`, so that
+ # `VariantVal.toJson` and the `to_json` SQL function produce the same
output.
+
+ @classmethod
+ def _float_to_json(cls, f: float, is_float32: bool) -> str:
+ """
+ Formats a double or float like Java's `Double.toString` and
`Float.toString`. Non-finite
+ values are quoted because they are not valid JSON numbers.
+ """
+ if math.isnan(f):
+ return '"NaN"'
+ if math.isinf(f):
+ return '"Infinity"' if f > 0 else '"-Infinity"'
+
+ def round_trips(s: str) -> bool:
+ if not is_float32:
+ return float(s) == f
+ try:
+ return struct.pack("<f", float(s)) == struct.pack("<f", f)
+ except OverflowError:
+ return False
+
+ # Like Java, pick the shortest decimal that round-trips, using at
least 2 significant
+ # digits (e.g. Double.MIN_VALUE is "4.9E-324", not "5.0E-324"), and
the one closest to
+ # `f` among those of that length.
+ for precision in range(2, 18):
+ s = "%.*e" % (precision - 1, f)
+ if round_trips(s):
+ break
+ # When `f` is a power of two, the values that round to it extend
only half as far
+ # toward zero as away from it. So the closest decimal of this
length can fail to
+ # round-trip while the next one away from zero still does (e.g.
2^-24 is
+ # "5.960464477539063E-8", not "5.9604644775390625E-8").
+ closest = decimal.Decimal(s)
+ exponent = closest.as_tuple().exponent
+ assert isinstance(exponent, int)
+ last_digit = decimal.Decimal(1).scaleb(exponent)
+ s = str(closest + last_digit if f > 0 else closest - last_digit)
+ if round_trips(s):
+ break
+ d = decimal.Decimal(s).normalize()
Review Comment:
**Non-blocking (P2):** This `normalize()`, and the `closest + last_digit` /
`closest - last_digit` retry above it, run in the caller's thread-local
`decimal.getcontext()`. If the application has lowered the precision (e.g.
`decimal.getcontext().prec = 5`, which is common in money code), `toJson()`
silently rounds the digits. I checked on this head: `_float_to_json(123456.789,
False)` returns `'123460.0'` and `_float_to_json(2.0**-24, False)` returns
`'5.9605E-8'`. The output is still valid JSON, so nothing signals it, and it no
longer matches `to_json`. Before this PR, floats went through `repr()`, which
ignores the decimal context.
`_decimal_to_json` below already avoids this with an explicit
`decimal.Context(prec=VariantUtils.MAX_DECIMAL16_PRECISION)`. Using the same
here (e.g. `ctx.add`/`ctx.subtract` and `normalize(ctx)`, or `with
decimal.localcontext(ctx):`) keeps the digits independent of caller state.
**Verification:**
- **Regression:** Under a lowered thread-local decimal precision, double and
float variant `toJson()` output equals the default-context output for
representative plain, E-notation, and power-of-two values. Restore the caller's
context afterwards.
- **Compatibility:** The existing JVM-format vectors (non-finite, signed
zero, thresholds, Double.MIN_VALUE, Float.MAX_VALUE, decimals, timestamps)
still produce the same strings.
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,93 @@ def handle_array(value_pos_list: List[int]) -> str:
if isinstance(value, bytes):
# decoding simply converts byte array to string
return '"' + base64.b64encode(value).decode("utf-8") + '"'
- if isinstance(value, (datetime.date, datetime.datetime)):
+ if isinstance(value, float):
+ return cls._float_to_json(value, is_float32)
+ if isinstance(value, decimal.Decimal):
+ return cls._decimal_to_json(value)
+ if isinstance(value, datetime.datetime):
+ return '"' + cls._timestamp_to_json(value) + '"'
+ if isinstance(value, datetime.date):
return '"' + str(value) + '"'
return str(value)
+ # The helpers below format scalars the same way as the JVM
`Variant.toJson`, so that
+ # `VariantVal.toJson` and the `to_json` SQL function produce the same
output.
+
+ @classmethod
+ def _float_to_json(cls, f: float, is_float32: bool) -> str:
+ """
+ Formats a double or float like Java's `Double.toString` and
`Float.toString`. Non-finite
+ values are quoted because they are not valid JSON numbers.
+ """
+ if math.isnan(f):
+ return '"NaN"'
+ if math.isinf(f):
+ return '"Infinity"' if f > 0 else '"-Infinity"'
+
+ def round_trips(s: str) -> bool:
+ if not is_float32:
+ return float(s) == f
+ try:
+ return struct.pack("<f", float(s)) == struct.pack("<f", f)
+ except OverflowError:
+ return False
+
+ # Like Java, pick the shortest decimal that round-trips, using at
least 2 significant
+ # digits (e.g. Double.MIN_VALUE is "4.9E-324", not "5.0E-324"), and
the one closest to
+ # `f` among those of that length.
+ for precision in range(2, 18):
+ s = "%.*e" % (precision - 1, f)
+ if round_trips(s):
+ break
+ # When `f` is a power of two, the values that round to it extend
only half as far
+ # toward zero as away from it. So the closest decimal of this
length can fail to
+ # round-trip while the next one away from zero still does (e.g.
2^-24 is
+ # "5.960464477539063E-8", not "5.9604644775390625E-8").
+ closest = decimal.Decimal(s)
+ exponent = closest.as_tuple().exponent
+ assert isinstance(exponent, int)
+ last_digit = decimal.Decimal(1).scaleb(exponent)
+ s = str(closest + last_digit if f > 0 else closest - last_digit)
Review Comment:
**Nit (P3):** The `else closest - last_digit` arm for negative values isn't
exercised. All the new power-of-two vectors are positive (2^-24, 2^-44, float32
2^-96), so if this arm went the wrong way, -2^-24 would fall through to the
17-digit `-5.9604644775390625E-8` and no test would catch it. It's correct
today (`_float_to_json(-(2.0**-24), False)` gives `-5.960464477539063E-8`).
Adding a negative vector such as `(double(-(2.0**-24)),
"-5.960464477539063E-8")` would guard it.
**Verification:**
- **Regression:** A negative power-of-two double (and optionally a float32)
formats to the same shortest JVM digits as its positive counterpart, with a
leading minus sign.
--
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]
