HyukjinKwon commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4217837901
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,105 @@ 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` on JDK 19+, so
+ # that `VariantVal.toJson` and the `to_json` SQL function produce the same
output. On older
+ # JDKs, `to_json` can print a longer, still round-tripping digit string
for some doubles and
+ # floats (see `_float_to_json`).
+
+ @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` on JDK 19+,
+ which print the shortest digits (JDK-4511638). Older JDKs can print a
longer, still
+ round-tripping digit string for some values, e.g. 2^-24 as
5.9604644775390625E-8 instead
+ of 5.960464477539063E-8. 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)
Review Comment:
**Nit (P3):** This decides float32 membership by rounding the candidate to a
double first and then to float32. When the decimal is within half a double ulp
of the midpoint between two float32 values, `float(s)` lands exactly on that
midpoint (every float32 midpoint is an exact double). `struct.pack` then ties
to the even neighbor, so a shorter decimal that really rounds to the *other*
float32 gets accepted.
Checked against Temurin JDK 25:
| float32 bits | this PR | JDK 19+ `Float.toString` |
|---|---|---|
| `0x15ae43fe` | `7.038531E-26` | `7.0385313E-26` |
| `0x15ae43fd` | `7.0385307E-26` | `7.038531E-26` |
`Float.parseFloat("7.038531E-26")` returns `0x15ae43fd`, so the first string
names the neighbor. It's rare: no case in 300k random floats, and these two
came from a targeted search near midpoints. The output is still valid JSON, but
`to_json` parity and the float32 round-trip are lost for these values.
Rounding to a double can only cross the true answer when `float(s)` is
exactly a float32 midpoint, so resolving that one tie exactly fixes the whole
class. For example, compare `decimal.Decimal(s)` with
`decimal.Decimal(float(s))`, or use `fractions.Fraction`, to pick the side.
**Verification:**
- **Regression:** The two float32 values next to a midpoint (bits 0x15ae43fe
and 0x15ae43fd) format to the JDK 19+ Float.toString strings.
- **Compatibility:** The existing JVM-format and test_variant_type
expectations still pass unchanged: non-finite values, signed zero, plain/E
thresholds, powers of two in both signs, Float.MAX_VALUE, decimals and
timestamps.
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,105 @@ 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` on JDK 19+, so
+ # that `VariantVal.toJson` and the `to_json` SQL function produce the same
output. On older
+ # JDKs, `to_json` can print a longer, still round-tripping digit string
for some doubles and
+ # floats (see `_float_to_json`).
+
+ @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` on JDK 19+,
+ which print the shortest digits (JDK-4511638). Older JDKs can print a
longer, still
+ round-tripping digit string for some values, e.g. 2^-24 as
5.9604644775390625E-8 instead
+ of 5.960464477539063E-8. 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
+
+ # Use an explicit context so that the result does not depend on the
caller's decimal
+ # context, e.g. a lowered `decimal.getcontext().prec`.
+ context = decimal.Context(prec=VariantUtils.MAX_DECIMAL16_PRECISION)
+
+ # 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):
Review Comment:
**Nit (P3):** Every double now runs up to 16 `%e` format/parse steps, and
each failed step also builds `Decimal`s for the power-of-two retry. On this
head, 20k random doubles take about 32 us each, versus about 0.4 us with the
previous `repr()`. So `toJson()`/`str()` on variants with many doubles (e.g.
called per row in a UDF) gets roughly 75x slower for that part.
Two shortcuts keep the output identical:
- `repr(f)` is already the shortest round-tripping decimal, so for doubles
the loop can start at `max(2, <significant digits of repr(f)>)`.
- The away-from-zero retry can only succeed when `f` is a power of two
(otherwise the rounding interval is symmetric), so it can be gated on
`abs(math.frexp(f)[0]) == 0.5`.
With both, output was byte-identical on 44k doubles (random bit patterns,
every power of two in both signs, the 1e-3/1e7 boundaries) and 20k float32
values, at about 4 us per double.
**Verification:**
- **Compatibility:** The new formatter output equals the current output on a
broad sample: random doubles and float32 values, every power of two in both
formats and signs, and the 1e-3/1e7 boundaries. All existing JVM-format and
test_variant_type expectations pass unchanged.
- **Inspection:** A timing comparison on a batch of random doubles shows
per-value formatting cost substantially below the current implementation.
##########
python/pyspark/sql/tests/test_types.py:
##########
@@ -3864,6 +3864,70 @@ def test_struct_field_type_name(self):
struct_field = StructField("a", IntegerType())
self.assertRaises(TypeError, struct_field.typeName)
+ def test_variant_to_json_matches_jvm_format(self):
+ import decimal
+ import struct
+
+ def variant(type_info, payload):
+ return VariantVal(bytes([type_info << 2]) + payload, bytes([1, 0,
0]))
+
+ def double(x):
+ return variant(7, struct.pack("<d", x))
+
+ def float32(x):
+ return variant(14, struct.pack("<f", x))
+
+ def decimal4(unscaled, scale):
+ return variant(8, bytes([scale]) + struct.pack("<i", unscaled))
+
+ def timestamp(micros, ntz):
+ return variant(13 if ntz else 12, struct.pack("<q", micros))
+
+ # The expected values are what the JVM `to_json` returns for the same
variants on JDK 19+.
+ # On older JDKs, `to_json` can print longer digit strings for some
doubles and floats,
+ # e.g. the powers of two below.
+ for v, expected in [
+ (double(float("nan")), '"NaN"'),
+ (double(float("inf")), '"Infinity"'),
+ (double(float("-inf")), '"-Infinity"'),
+ (double(0.0), "0.0"),
+ (double(-0.0), "-0.0"),
+ (double(123.0), "123.0"),
+ (double(123456.789), "123456.789"),
+ (double(0.001), "0.001"),
+ (double(1e7), "1.0E7"),
+ (double(1.5e-5), "1.5E-5"),
+ (double(-1e20), "-1.0E20"),
+ (double(5e-324), "4.9E-324"),
+ # Powers of two whose shortest decimal is not the closest one of
that length.
+ (double(2.0**-24), "5.960464477539063E-8"),
+ (double(-(2.0**-24)), "-5.960464477539063E-8"),
+ (double(2.0**-44), "5.684341886080802E-14"),
+ (float32(2.0**-96), "1.2621775E-29"),
+ (float32(-(2.0**-96)), "-1.2621775E-29"),
+ (float32(float("nan")), '"NaN"'),
+ (float32(1.1), "1.1"),
+ (float32(1e10), "1.0E10"),
+ (float32(3.4028234663852886e38), "3.4028235E38"),
+ (decimal4(100, 2), "1"),
+ (decimal4(0, 2), "0"),
+ (decimal4(-1, 9), "-0.000000001"),
+ (timestamp(1_500_000, ntz=True), '"1970-01-01 00:00:01.5"'),
+ (timestamp(1, ntz=False), '"1970-01-01 00:00:00.000001+00:00"'),
+ ]:
+ with self.subTest(expected=expected):
+ self.assertEqual(v.toJson(), expected)
+ # The output must always be valid JSON.
+ json.loads(v.toJson())
+ # The output must not depend on the caller's decimal context.
+ with decimal.localcontext() as ctx:
+ ctx.prec = 5
+ self.assertEqual(v.toJson(), expected)
+
+ self.assertEqual(
+ timestamp(0, ntz=False).toJson("Asia/Kolkata"), '"1970-01-01
05:30:00+05:30"'
Review Comment:
**Nit (P3):** Every offset asserted here is a whole number of minutes
(`+00:00`, `-08:00`, `+05:30`), so the seconds-dropping rule this PR adds for
LMT offsets isn't covered. Printing the seconds again, or rounding instead of
truncating, would still pass. Similarly, the longest double expectation has 16
significant digits, so nothing reaches the 17-digit end of `range(2, 18)`.
Two vectors would pin both:
- 1800-01-01T00:00Z with `toJson("America/Los_Angeles")`, expecting
`"1799-12-31 16:07:02-07:52"` (the UTC offset there is -07:52:58).
- `double(0.1 + 0.2)`, expecting `"0.30000000000000004"`.
**Verification:**
- **Regression:** The test fails if _timestamp_to_json prints offset seconds
or rounds instead of truncating, and fails if the double digit search cannot
reach 17 digits.
- **Behavior:** The new vectors pass on the current implementation.
--
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]
