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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to