Re: [PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]

Fri, 01 Aug 2025 15:19:44 -0700 


korbit-ai[bot] commented on code in PR #34514:
URL: https://github.com/apache/superset/pull/34514#discussion_r2248949817


##########
superset/utils/json.py:
##########
@@ -155,9 +160,27 @@ def json_int_dttm_ser(obj: Any) -> Any:
     """
 
     if isinstance(obj, (datetime, pd.Timestamp)):
+        # Check if datetime is within JavaScript's safe date range
+        # If not, return ISO string instead of epoch milliseconds
+        if isinstance(obj, pd.Timestamp):
+            dttm = obj.to_pydatetime()
+        else:
+            dttm = obj
+
+        # Remove timezone info for comparison
+        dttm_no_tz = dttm.replace(tzinfo=None) if dttm.tzinfo else dttm

Review Comment:
   ### Timezone Stripping Affects Range Check Accuracy <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Timezone information is being stripped for comparison with 
JS_DATE_RANGE_MIN/MAX bounds, which could lead to incorrect range checks for 
timezone-aware timestamps.
   
   
   ###### Why this matters
   When timezone information is removed, the actual datetime value shifts, 
potentially causing valid dates to be misclassified as out of range or vice 
versa. This defeats the purpose of accurate datetime handling.
   
   ###### Suggested change ∙ *Feature Preview*
   Instead of stripping timezone info, convert the datetime to UTC for 
comparison:
   ```python
   dttm_utc = dttm.astimezone(timezone.utc) if dttm.tzinfo else dttm
   if dttm_utc < JS_DATE_RANGE_MIN or dttm_utc > JS_DATE_RANGE_MAX:
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/309c829e-d6b3-4419-9a58-98b6dcfc37c6/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/309c829e-d6b3-4419-9a58-98b6dcfc37c6?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/309c829e-d6b3-4419-9a58-98b6dcfc37c6?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/309c829e-d6b3-4419-9a58-98b6dcfc37c6?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/309c829e-d6b3-4419-9a58-98b6dcfc37c6)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:cd756ce4-2927-45e1-be68-a598b15f880f -->
   
   
   [](cd756ce4-2927-45e1-be68-a598b15f880f)



-- 
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