You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2025/08/01 22:16:00 UTC
[PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]
rusackas opened a new pull request, #34514:
URL: https://github.com/apache/superset/pull/34514
### SUMMARY
This PR fixes an issue where datetime values outside JavaScript's safe date range were returning "0NaN-NaN-NaN" in charts when using Microsoft SQL Server.
The root cause was that MSSQL supports a much wider datetime range (1753-9999) than JavaScript's Date object can handle (approximately 1938-2286). When dates outside this range were converted to epoch milliseconds, they would overflow JavaScript's Number type, resulting in NaN values that displayed as "0NaN-NaN-NaN" in charts.
**Changes:**
- Added constants defining JavaScript's safe date range in `superset/utils/dates.py`
- Modified `json_int_dttm_ser` to return ISO strings for dates outside the safe range
- Added comprehensive tests for edge cases
- Maintains backward compatibility for dates within the normal range
Fixes #27790
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before:** Charts display "0NaN-NaN-NaN" for datetime values outside JavaScript's safe range
**After:** Charts display the actual datetime values correctly as ISO strings
### TESTING INSTRUCTIONS
1. Connect to a Microsoft SQL Server database
2. Create a query with datetime values outside JavaScript's safe range:
```sql
SELECT
CAST('1938-04-24 22:13:20.002' AS DATETIME) AS early_date,
CAST('2286-11-20 17:46:39.998' AS DATETIME) AS late_date,
CAST('2000-01-01 12:00:00' AS DATETIME) AS normal_date
```
3. Create a chart using these datetime columns
4. Verify that:
- Early and late dates display as proper datetime strings (not "0NaN-NaN-NaN")
- Normal dates continue to work as before
- No JavaScript console errors appear
### ADDITIONAL INFORMATION
- [x] Has associated issue: #27790
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.ai/code)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #34514: fix(charts): handle MSSQL datetime values outside JavaScript's safe range
URL: https://github.com/apache/superset/pull/34514
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
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></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
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/309c829e-d6b3-4419-9a58-98b6dcfc37c6/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/309c829e-d6b3-4419-9a58-98b6dcfc37c6?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/309c829e-d6b3-4419-9a58-98b6dcfc37c6?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/309c829e-d6b3-4419-9a58-98b6dcfc37c6?what_not_in_standard=true) [](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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34514:
URL: https://github.com/apache/superset/pull/34514#discussion_r2249434864
##########
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
+
+ if dttm_no_tz < JS_DATE_RANGE_MIN or dttm_no_tz > JS_DATE_RANGE_MAX:
Review Comment:
Anyhow we're mostly good until 2286 :/
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #34514:
URL: https://github.com/apache/superset/pull/34514#issuecomment-3146803650
Would have to dig deeper, but seems dangerous to introduce mixed types for date field in json across the board (?) Might need to be tested across plugins / explore/ sql lab. Feels like the right solution might be to use ISO consistently all the time? Would/could that break external plugins for instance? Almost feel like we'd need a new `api/v3/` endpoints that switch to ISO, and retrofit the plugins to use it?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #34514:
URL: https://github.com/apache/superset/pull/34514#discussion_r2252345527
##########
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
+
+ if dttm_no_tz < JS_DATE_RANGE_MIN or dttm_no_tz > JS_DATE_RANGE_MAX:
+ # Return ISO string for dates outside JavaScript's safe range
+ return obj.isoformat()
+
return datetime_to_epoch(obj)
if isinstance(obj, date):
+ # Check if date is within JavaScript's safe date range
Review Comment:
Here's what _le bot_ says:
<img width="712" height="654" alt="image" src="https://github.com/user-attachments/assets/4c4d6c25-288a-412e-a958-dc16a15dfc9c" />
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34514:
URL: https://github.com/apache/superset/pull/34514#discussion_r2249434401
##########
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
+
+ if dttm_no_tz < JS_DATE_RANGE_MIN or dttm_no_tz > JS_DATE_RANGE_MAX:
+ # Return ISO string for dates outside JavaScript's safe range
+ return obj.isoformat()
+
return datetime_to_epoch(obj)
if isinstance(obj, date):
+ # Check if date is within JavaScript's safe date range
Review Comment:
Oh interesting, it would have been an issue before this PR, but does `date.date` not have a tzinfo like `date.datetime`? Seem we should apply this consistently ...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34514:
URL: https://github.com/apache/superset/pull/34514#discussion_r2249434760
##########
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
+
+ if dttm_no_tz < JS_DATE_RANGE_MIN or dttm_no_tz > JS_DATE_RANGE_MAX:
Review Comment:
So this means our JSON serializer tries to convert to epoch, and will return mixed types for dates (string or int depending on whether it fits in an int?) Wondering if it's handled properly in the frontend...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(charts): handle MSSQL datetime values outside JavaScript's safe range [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #34514:
URL: https://github.com/apache/superset/pull/34514#issuecomment-3237817565
Maybe I didn't understand https://github.com/apache/superset/issues/27790 but I just tried to reproduce and couldn't, so I closed that issue. This PR was linked to that issue so I'm mentioning it here.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org