You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "jubins (via GitHub)" <gi...@apache.org> on 2026/10/02 20:22:25 UTC
[PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
jubins opened a new pull request, #59214:
URL: https://github.com/apache/spark/pull/59214
### What changes were proposed in this pull request?
Fixes [SPARK-59910](https://issues.apache.org/jira/browse/SPARK-59910). `VariantVal.toJson()` (and `str(VariantVal)`) is implemented in pure Python in `pyspark/sql/variant_utils.py`. It formatted scalars with Python's `str()`/`repr()`, which differs from the JVM implementation (`Variant.toJsonImpl` in `common/variant`). For non-finite doubles and floats it produced output that is not valid JSON.
This PR formats scalars the same way as the JVM:
- **Doubles and floats**: NaN and infinities are quoted (`"NaN"`, `"Infinity"`, `"-Infinity"`), as in the JVM. Finite values are formatted like Java's `Double.toString` / `Float.toString`:
- the shortest correctly rounded decimal that round-trips, with at least 2 significant digits;
- plain notation for magnitudes in [1e-3, 1e7), and `E` notation otherwise.
A 4-byte FLOAT is formatted using float32 precision rather than as the widened double.
- **Decimals**: formatted like `BigDecimal.stripTrailingZeros().toPlainString()`, with no trailing zeros and never scientific notation.
- **Timestamps**: the fractional seconds have no trailing zeros, and the UTC offset is printed as `+HH:MM`, dropping the seconds of historical local-mean-time offsets, as `DateTimeFormatter` does.
`toPython()` is unchanged.
**Brief change log**
- `python/pyspark/sql/variant_utils.py`: adds `_float_to_json`, `_decimal_to_json` and `_timestamp_to_json`, and uses them in `_to_json`.
- `python/pyspark/sql/tests/test_types.py`:
- Adds `test_variant_to_json_matches_jvm_format`, which builds variant bytes directly and checks each formatting rule and that the output parses as JSON.
- Updates `test_variant_type` expectations that had pinned down the old output.
### Why are the changes needed?
`toJson()` can return invalid JSON, and it returns different text than `to_json` for the same variant:
```python
spark.conf.set("spark.sql.session.timeZone", "UTC")
e = "cast(double('NaN') as variant)"
r = spark.sql(f"select {e} v, to_json({e}) j").first()
r.j # '"NaN"'
r.v.toJson() # 'nan' -> json.loads() fails
```
| Value | JVM `to_json` | Before | After |
|---|---|---|---|
| double NaN | `"NaN"` | `nan` (invalid JSON) | `"NaN"` |
| double Infinity | `"Infinity"` | `inf` (invalid JSON) | `"Infinity"` |
| double 1e7 | `1.0E7` | `10000000.0` | `1.0E7` |
| float 1.1 | `1.1` | `1.100000023841858` | `1.1` |
| decimal(5,2) 1.00 | `1` | `1.00` | `1` |
| decimal(10,9) 0.000000001 | `0.000000001` | `1E-9` | `0.000000001` |
| timestamp_ntz `1970-01-01 00:00:01.5` | `"1970-01-01 00:00:01.5"` | `"1970-01-01 00:00:01.500000"` | `"1970-01-01 00:00:01.5"` |
| timestamp `1800-01-01 00:00:00` (America/Los_Angeles) | `"...-07:52"` | `"...-07:52:58"` | `"...-07:52"` |
### Does this PR introduce _any_ user-facing change?
Yes. The strings returned by `VariantVal.toJson()` and `str(VariantVal)` change for doubles, floats, decimals and timestamps, as shown in the table above. They now match what the `to_json` SQL function returns for the same value, and non-finite numbers no longer produce invalid JSON. The behaviour has been in place since `VariantVal` was added in 4.0.0.
### How was this patch tested?
- `DataTypeTests.test_variant_to_json_matches_jvm_format` (new, no JVM needed). 15 of its cases fail without this change.
- `TypesTests.test_variant_type` (updated expectations).
- A differential check against the JVM, not included in the PR: 4,623 variant values in each of 4 session time zones (UTC, America/Los_Angeles, Asia/Kolkata, Asia/Kathmandu), 18,492 comparisons of `VariantVal.toJson()` with the `to_json` result for the same value. The values included:
- 2,000 random doubles and 2,000 random floats (including subnormals and extremes);
- 300 random decimals of up to 28 digits;
- 300 random timestamps across years 1 to 9999, plus nested arrays and objects.
All values matched, except a 29-digit decimal. That value is affected by a separate precision issue in `_get_decimal`: it rounds to Python's default 28-digit decimal context, which also affects `toPython()`. That issue is out of scope for this PR.
The check ran on JDK 21. The float formatting follows the `Double.toString` specification of JDK 19 and later; earlier JDKs can print a longer digit string for a small number of values.
```
python -m pytest pyspark/sql/tests/test_types.py pyspark/sql/tests/connect/test_parity_types.py
```
218 passed. `TypesParityTests.test_infer_schema_specification` fails, but it fails the same way on master without this change (a `TIME` type error unrelated to variants).
`ruff check`, `ruff format --check` and `mypy` pass.
### Was this patch authored or co-authored using generative AI tooling?
- [x] Yes - Claude Code was used as a pair-programming assistant. All changes were reviewed and verified by the author. Generated-by: Claude Opus 5.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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4171645550
##########
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:
added negative double and float vectors.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6011948723
hi @cloud-fan @HyukjinKwon please let me know if there are further comments, would appreciate if this can be approved/merged, if nothing else. thank you!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4247110592
##########
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:
Added both, `0.1 + 0.2` and the 1800-01-01 Los Angeles LMT case.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4255305044
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,127 @@ 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:
+ x = float(s)
+ if not is_float32:
+ return x == f
+ try:
+ rounded = struct.unpack("<f", struct.pack("<f", x))[0]
+ except OverflowError:
+ return False
+ if rounded != x:
+ # Rounding the decimal to a double first can change which float32 it rounds to
+ # only when the double is exactly the midpoint between two float32 values. Every
+ # such midpoint is an exact double, so resolve that tie with the exact decimal.
+ bits = struct.unpack("<I", struct.pack("<f", abs(rounded)))[0]
+ bits = bits + 1 if abs(x) > abs(rounded) else bits - 1
+ other = math.copysign(struct.unpack("<f", struct.pack("<I", bits))[0], rounded)
+ midpoint = (rounded + other) / 2
+ if x == midpoint:
+ exact, exact_midpoint = decimal.Decimal(s), decimal.Decimal(midpoint)
Review Comment:
Fixed, it now uses `Decimal.from_float`, and the test runs every case with the `FloatOperation` trap on.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4248728844
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,127 @@ 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:
+ x = float(s)
+ if not is_float32:
+ return x == f
+ try:
+ rounded = struct.unpack("<f", struct.pack("<f", x))[0]
+ except OverflowError:
+ return False
+ if rounded != x:
+ # Rounding the decimal to a double first can change which float32 it rounds to
+ # only when the double is exactly the midpoint between two float32 values. Every
+ # such midpoint is an exact double, so resolve that tie with the exact decimal.
+ bits = struct.unpack("<I", struct.pack("<f", abs(rounded)))[0]
+ bits = bits + 1 if abs(x) > abs(rounded) else bits - 1
+ other = math.copysign(struct.unpack("<f", struct.pack("<I", bits))[0], rounded)
+ midpoint = (rounded + other) / 2
+ if x == midpoint:
+ exact, exact_midpoint = decimal.Decimal(s), decimal.Decimal(midpoint)
Review Comment:
**Nit (P3):** `decimal.Decimal(midpoint)` converts a float through the implicit constructor, which consults the caller's thread-local decimal context, not the explicit `context` created below. If an application turns on Python's strict float/Decimal mode (`decimal.getcontext().traps[decimal.FloatOperation] = True`), `toJson()`/`str()` raise `decimal.FloatOperation` here instead of returning JSON.
I checked this on the current head:
- The test's own vectors `0x15AE43FE` and `0x15AE43FD` raise.
- 12,609 of 20,000 sampled float32 values in [1e6, 1e9) raise, and none below 1e6 do.
- Doubles never reach this branch.
Before this PR, floats never touched `decimal`, so this exception path is new. Even with the trap off, the branch sets the `FloatOperation` flag on the caller's context.
`decimal.Decimal.from_float(midpoint)` is an exact, explicit conversion that the trap allows. Swapping it in removed every exception and left the output unchanged on 100,000 samples. My earlier `decimal.Decimal(float(s))` suggestion had the same problem, sorry about that.
**Verification:**
- **Regression:** Under a caller context with the FloatOperation trap enabled, the float32 midpoint vectors and a float32 value in the millions format to their expected JDK 19+ strings instead of raising.
- **Compatibility:** All existing JVM-format vectors and test_variant_type expectations still produce the same strings under the default and lowered-precision contexts.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6125568620
Thanks for the updates. I rechecked the latest commit and all three earlier nits look addressed: the float32 midpoint tie is now resolved against the exact decimal, the double search starts from repr()'s digit count with the power-of-two retry gated on frexp, and the test now pins 0.1 + 0.2 and the 1800-01-01 Los Angeles LMT offset. The review posted alongside this reply has the details.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"conversation:6122673107","thread_id":"conversation:6122673107","verdict_sha256":"ef286150477e4f71e814af23255a2ae5ff3430e17902ccea5c6410200e21ecc9"} -->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4169781569
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,81 @@ 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 correctly rounded decimal that round-trips, using at least
+ # 2 significant digits (e.g. Double.MIN_VALUE is "4.9E-324", not "5.0E-324").
+ for precision in range(2, 18):
Review Comment:
**Nit (P3):** This only tries the correctly rounded `precision`-digit decimal. When `f` is a power of two, the rounding interval below `f` is half as wide as the one above it. So the nearest n-digit decimal can fail to round-trip while the next n-digit decimal away from zero still does, and JDK 19+ `Double.toString`/`Float.toString` pick that one.
I checked against Temurin JDK 25 using every double and float power of two plus 200k random bit patterns of each. The random values all match, but 46 double powers of two and 3 float powers of two come out one digit longer than the JVM:
| value | JVM `to_json` | this PR |
|---|---|---|
| double 2^-24 | `5.960464477539063E-8` | `5.9604644775390625E-8` |
| double 2^-44 | `5.684341886080802E-14` | `5.6843418860808015E-14` |
| float 2^-96 | `1.2621775E-29` | `1.26217745E-29` |
(Before this PR, `str()`/repr already printed the 16-digit form for these doubles.) The output still parses to the same value, so only parity with `to_json` is affected. When the rounded candidate fails, also trying the decimal one unit in the last place away from zero at the same precision before increasing `precision` fixed all 49 cases, with no other output changes across the 401,418 values checked.
**Verification:**
- **Regression:** Assert that power-of-two doubles and floats whose shortest decimal lies above the nearest rounding (e.g. 2^-24 double, 2^-96 float) format to the JDK 19+ strings.
- **Compatibility:** Existing JVM-format and variant type expectations (NaN/Infinity, signed zero, plain/E boundaries, Double.MIN_VALUE, Float.MAX_VALUE, decimals, timestamps) still pass unchanged.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6340687595
Thanks for the ping, and sorry for the wait. I took another pass over the latest commit; the review posted alongside this reply has the details.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"conversation:6335127669","thread_id":"conversation:6335127669","verdict_sha256":"f9e1058f6e87b76f2bb6a14a673a6aca69e0f1770f851744da5ba74e949e08b6"} -->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6125738543
Thanks - I rechecked the fixes discussed in the prior rounds, and the current head includes them.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"conversation:6122673107","thread_id":"conversation:6122673107","verdict_sha256":"ac5fdd10b02627e060cf78441738f54a85073cf4642d6dd666d9d5262f58ab6f"} -->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6139479356
> Thanks for the updates. I rechecked the latest commit and all three earlier nits look addressed: the float32 midpoint tie is now resolved against the exact decimal, the double search starts from repr()'s digit count with the power-of-two retry gated on frexp, and the test now pins 0.1 + 0.2 and the 1800-01-01 Los Angeles LMT offset. The review posted alongside this reply has the details.
Thanks @HyukjinKwon! Addressed the `FloatOperation` nit as well. could you please take another look?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6160891361
> @jubins did you push the new commit?
@cloud-fan forgot to push the commit, just pushed, can you please take a look?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4348271480
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,130 @@ 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
Review Comment:
**Nit (P3):** Older JDKs don't only print longer strings. For some values they print different digits of the same length. I checked locally on OpenJDK 17.0.15 and Temurin 25:
| value | JDK 17 | JDK 25 / this PR |
|---|---|---|
| `Double.longBitsToDouble(0x14)` | `1.0E-322` | `9.9E-323` |
| `Float.intBitsToFloat(0x6a277b57)` | `5.0618266E25` | `5.0618267E25` |
| `Float.intBitsToFloat(0xea06438d)` | `-4.0578764E25` | `-4.0578765E25` |
Spark still supports Java 17, so someone comparing against a Java 17 cluster could hit one of these and read it as a bug in the port. Could you describe it as "a different, still round-tripping digit string (sometimes longer)" here, in the `_float_to_json` docstring, and in the expected-value comment in the new test? This is text-only; no behavior change is needed.
This extends an [existing discussion](https://github.com/apache/spark/pull/59214#discussion_r4171795926).
**Verification:**
- **Inspection:** The helper section comment, the _float_to_json docstring and the JVM-format test comment describe older-JDK output as a possibly different, still round-tripping digit string, consistent with JDK 17 printing 1.0E-322 where the JDK 19+ form is 9.9E-323.
- **Compatibility:** Every existing JVM-format vector and variant type expectation is unchanged, showing that the correction is text-only.
##########
python/pyspark/sql/tests/test_types.py:
##########
@@ -3864,6 +3864,83 @@ 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"),
+ (double(0.1 + 0.2), "0.30000000000000004"),
+ # 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"),
+ # Neighboring floats whose shortest decimals, read as doubles, are exactly the
+ # midpoint between them.
+ (variant(14, struct.pack("<I", 0x15AE43FE)), "7.0385313E-26"),
+ (variant(14, struct.pack("<I", 0x15AE43FD)), "7.038531E-26"),
+ (float32(33554448.0), "3.355445E7"),
+ (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, and must work when
Review Comment:
**Nit (P3):** This comment, and the section comment above the helpers in `variant_utils.py` ("so that `VariantVal.toJson` and the `to_json` SQL function produce the same output"), promise more than the change delivers for DECIMAL leaves. `_decimal_to_json` normalizes under its own context, but its input comes from the unchanged `_get_decimal`, which computes `Decimal(unscaled) * Decimal(10) ** -scale` in the caller's context. On this head, a DECIMAL8 leaf with unscaled 123456789 and scale 2 prints `1234567.89` by default but `1234600` under `prec = 5`. A 38-digit DECIMAL16 also loses digits under the default 28-digit context (`123456789012345678901234567900000000`), while `to_json` keeps all of them. The loop below can't catch this, because every decimal vector has at most three significant digits.
The digit loss itself is the same as on master, and the PR description already puts `_get_decimal` out of scope, so I'm not asking for that fix here. Could you narrow the two comments instead? For example, say that the lowered-context re-check covers the new formatting, and note in the section comment that decimal leaves are decoded in the caller's decimal context, so values wider than its precision (28 digits by default) can still differ from `to_json`.
**Verification:**
- **Inspection:** The helper section comment and the JVM-format test comment limit their context-independence and to_json-parity claims to the formatting this change adds, and state that decimal leaves decoded in the caller's decimal context can still lose digits beyond its precision.
- **Compatibility:** Every existing JVM-format vector and the updated variant type expectations still produce the same strings under the default context, a lowered precision, and an enabled FloatOperation trap.
##########
python/pyspark/sql/tests/test_types.py:
##########
@@ -3864,6 +3864,83 @@ 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"),
+ (double(0.1 + 0.2), "0.30000000000000004"),
+ # 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"),
+ # Neighboring floats whose shortest decimals, read as doubles, are exactly the
Review Comment:
**Nit (P3):** Only one of these two floats has a shortest decimal that lands on the midpoint. `float("7.038531e-26")` equals the double midpoint of 0x15AE43FD and 0x15AE43FE, but `float("7.0385313e-26")` does not. What the pair actually tests is that both floats share the closest 7-digit decimal, `7.038531E-26`. Read as a double it is exactly the midpoint, and its exact value rounds to 0x15AE43FD. So the tie-break has to accept it for 0x15AE43FD and reject it for 0x15AE43FE, which then needs 8 digits. Could you reword the comment along those lines? The expected strings themselves are right.
This extends an [existing discussion](https://github.com/apache/spark/pull/59214#discussion_r4217837901).
**Verification:**
- **Inspection:** The comment above the float32 midpoint vectors says that only the shared 7-digit candidate 7.038531E-26 reads as the midpoint double, that it names the lower float, and that the upper float therefore needs 8 digits.
- **Compatibility:** The float32 midpoint vectors still expect 7.0385313E-26 and 7.038531E-26 and pass unchanged.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4171867080
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,100 @@ 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
Review Comment:
done, the three comments now say JDK 19+ and note that older JDKs can print longer digits.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4362851462
##########
python/pyspark/sql/tests/test_types.py:
##########
@@ -3864,6 +3864,83 @@ 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"),
+ (double(0.1 + 0.2), "0.30000000000000004"),
+ # 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"),
+ # Neighboring floats whose shortest decimals, read as doubles, are exactly the
+ # midpoint between them.
+ (variant(14, struct.pack("<I", 0x15AE43FE)), "7.0385313E-26"),
+ (variant(14, struct.pack("<I", 0x15AE43FD)), "7.038531E-26"),
+ (float32(33554448.0), "3.355445E7"),
+ (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, and must work when
Review Comment:
Narrowed both comments: they now say decimal leaves are still decoded in the caller's context, so values wider than its precision can differ from `to_json`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4171447562
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,81 @@ 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 correctly rounded decimal that round-trips, using at least
+ # 2 significant digits (e.g. Double.MIN_VALUE is "4.9E-324", not "5.0E-324").
+ for precision in range(2, 18):
Review Comment:
@HyukjinKwon thanks for the careful check! Fixed if the closest decimal doesn't round-trip, it now tries the next one away from zero before adding a digit. All double and float powers of two now match `to_json`, and I added `2^-24`, `2^-44` and `2^-96` as test cases.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4171644612
##########
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:
good catch @HyukjinKwon, fixed, now using an explicit `decimal.Context` like `_decimal_to_json`, and the test also checks every vector under a lowered `prec`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4171795926
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,100 @@ 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
Review Comment:
**Nit (P3):** This comment, the `_float_to_json` docstring below, and the test comment `The expected values are what the JVM to_json returns` state parity with the JVM unconditionally. `Double.toString`/`Float.toString` only became shortest-digit in JDK 19 (JDK-4511638), and Spark still runs on Java 17 (`docs/index.md`). I checked on Temurin 17 vs 25:
| value | JDK 17 `to_json` | JDK 19+ / this PR |
|---|---|---|
| double 2^-24 | `5.9604644775390625E-8` | `5.960464477539063E-8` |
| double 2^-44 | `5.6843418860808015E-14` | `5.684341886080802E-14` |
| float 2^-96 | `1.26217745E-29` | `1.2621775E-29` |
| double 2e23 | `1.9999999999999998E23` | `2.0E23` |
So on a Java 17 deployment, the new power-of-two test vectors are not what `to_json` returns. The PR description already says this ("follows the `Double.toString` specification of JDK 19 and later"), but that boundary isn't recorded in the code. Someone who later compares against a Java 17 cluster could "fix" the retry back to the longer strings. Could you mention JDK 19+ in these three comments, e.g. "formats like JDK 19+ `Double.toString`; older JDKs can print a longer, still round-tripping digit string for some values"? No behavior change is needed.
**Verification:**
- **Inspection:** The helper block comment, the _float_to_json docstring and the test expected-value comment qualify JVM parity with the JDK 19+ specification and mention the possible pre-JDK-19 divergence.
- **Compatibility:** All existing variant toJson vectors and test_variant_type expectations still pass unchanged, showing the change is text-only.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6261385097
@cloud-fan thanks for the approval, is this good to merge? thanks for the reviews
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6371761271
> Thanks for the ping, and sorry for the wait. I took another pass over the latest commit; the review posted alongside this reply has the details.
Thanks @cloud-fan! All four wording nits are fixed. On the migration guide: yes, I've added a one-line entry under a new "Upgrading from PySpark 4.3 to 4.4" section, since this first ships in 4.4.0.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-5981157662
@HyukjinKwon does the changes look good? let me know if you have further comments
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6054668293
Hi @uros-b @cloud-fan @HyukjinKwon please let me know if any further feedback, would appreciate if this is good to merge
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6207827182
@cloud-fan could you please review?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4247108318
##########
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:
both shortcuts applied, output is unchanged on ~500k doubles and floats, and a double formats about 5.7x faster.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4247100742
##########
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:
fixed, the midpoint tie is now resolved with the exact decimal
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4248827781
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,127 @@ 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:
+ x = float(s)
+ if not is_float32:
+ return x == f
+ try:
+ rounded = struct.unpack("<f", struct.pack("<f", x))[0]
+ except OverflowError:
+ return False
+ if rounded != x:
+ # Rounding the decimal to a double first can change which float32 it rounds to
+ # only when the double is exactly the midpoint between two float32 values. Every
+ # such midpoint is an exact double, so resolve that tie with the exact decimal.
+ bits = struct.unpack("<I", struct.pack("<f", abs(rounded)))[0]
+ bits = bits + 1 if abs(x) > abs(rounded) else bits - 1
+ other = math.copysign(struct.unpack("<f", struct.pack("<I", bits))[0], rounded)
+ midpoint = (rounded + other) / 2
+ if x == midpoint:
+ exact, exact_midpoint = decimal.Decimal(s), decimal.Decimal(midpoint)
+ if exact != exact_midpoint and (exact < exact_midpoint) != (rounded < midpoint):
+ rounded = other
+ return rounded == f
+
+ # 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 a double, repr() already has the shortest length,
+ # so start there instead of trying every shorter one.
+ start = 2
+ if not is_float32:
+ shortest = decimal.Decimal(repr(f)).normalize(context)
+ start = max(start, len(shortest.as_tuple().digits))
+ for precision in range(start, 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"). Elsewhere the interval is
+ # symmetric and the next one cannot round-trip either.
+ if abs(math.frexp(f)[0]) != 0.5:
+ continue
+ closest = decimal.Decimal(s)
+ exponent = closest.as_tuple().exponent
+ assert isinstance(exponent, int)
+ last_digit = decimal.Decimal(1).scaleb(exponent, context)
+ if f > 0:
+ s = str(context.add(closest, last_digit))
+ else:
+ s = str(context.subtract(closest, last_digit))
+ if round_trips(s):
+ break
+ d = decimal.Decimal(s).normalize(context)
+ if d.is_zero() or 1e-3 <= abs(f) < 1e7:
+ plain = format(d, "f")
+ return plain if "." in plain else plain + ".0"
+ sign, digits, exponent = d.as_tuple()
+ assert isinstance(exponent, int)
+ mantissa = str(digits[0]) + "." + ("".join(map(str, digits[1:])) or "0")
+ return ("-" if sign else "") + mantissa + "E" + str(len(digits) - 1 + exponent)
+
+ @classmethod
+ def _decimal_to_json(cls, d: decimal.Decimal) -> str:
+ """
+ Formats a decimal like Java's `BigDecimal.stripTrailingZeros().toPlainString()`.
+ """
+ context = decimal.Context(prec=VariantUtils.MAX_DECIMAL16_PRECISION)
+ return format(d.normalize(context), "f")
+
+ @classmethod
+ def _timestamp_to_json(cls, ts: datetime.datetime) -> str:
+ """
+ Formats a timestamp as "yyyy-MM-dd HH:mm:ss[.fraction][+HH:MM]", where the fraction has
Review Comment:
**Nit (P3):** The optional offset is documented as `[+HH:MM]`, but the branch below emits `-` for negative zones and the new Los Angeles LMT vector expects `-07:52`. Please describe this as a signed `HH:MM` offset so the documented grammar covers both forms.
**Verification:**
- **Inspection:** The helper contract explicitly describes both offset signs and remains consistent with the conditional timezone-aware branch.
- **Regression:** Representative positive and negative timezone offsets retain their existing minute-resolution JSON spellings.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6122673107
> Thanks for the ping, and sorry for the wait. I took another pass over the latest commit; the review posted alongside this reply has the details.
Thanks for the review @HyukjinKwon! I've addressed all three nits. Could you take another look when you have a chance?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6139576534
@jubins did you push the new commit?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6057948257
Thanks for the ping, and sorry for the wait. I took another pass over the latest commit; the review posted alongside this reply has the details.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"conversation:6054668293","thread_id":"conversation:6054668293","verdict_sha256":"157e983ad3b8031107c4c58062f66791b0d2dd5367ed4dcf1d8d898d8a6a4d34"} -->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6299951412
@cloud-fan @HyukjinKwon is this good to merge?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on PR #59214:
URL: https://github.com/apache/spark/pull/59214#issuecomment-6335127669
hi @cloud-fan is this good to merge? please let me know if further comments
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59910][PYTHON] Make VariantVal.toJson produce valid JSON and match the JVM to_json output [spark]
Posted by "jubins (via GitHub)" <gi...@apache.org>.
jubins commented on code in PR #59214:
URL: https://github.com/apache/spark/pull/59214#discussion_r4362853159
##########
python/pyspark/sql/tests/test_types.py:
##########
@@ -3864,6 +3864,83 @@ 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"),
+ (double(0.1 + 0.2), "0.30000000000000004"),
+ # 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"),
+ # Neighboring floats whose shortest decimals, read as doubles, are exactly the
Review Comment:
Reworded: both floats share the closest 7-digit decimal 7.038531E-26, which is exactly the midpoint and names the lower float, so the upper one needs 8 digits.
##########
python/pyspark/sql/variant_utils.py:
##########
@@ -413,10 +417,130 @@ 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
Review Comment:
Updated all three to say older JDKs can print a different, still round-tripping digit string (sometimes longer).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org