fix(dynamodb): render time bounds as ISO 8601 so sub-day ranges match stored timestamps

[aminghadersohi]

SUMMARY

DynamoDB has no datetime type. Timestamps are stored as strings and compared as text, conventionally in ISO 8601 form (2019-01-02T04:15:00), which is also what PyDynamoDB writes for datetime values. DynamoDBEngineSpec.convert_dttm rendered time-range bounds with a space separator ('2019-01-02 04:00:00'). Because " " sorts before "T", every row on the boundary day compared as greater than any bound on that day. As a result, a sub-day time range (for example 04:00–06:00) excluded matching rows, and an hour-range chart returned no rows.

This PR renders the bounds with dttm.isoformat(), e.g. 2019-01-02T04:00:00, with fractional seconds kept when set, so sub-second bounds are not truncated. For ISO-formatted stored values, day-granular ranges are unaffected. Space-separated stored timestamps must be normalized before upgrading; otherwise whole-day ranges can shift as well. UPDATING.md and the user guide document this compatibility change. boto3 itself does not serialize datetime objects. The SQLite time-grain expressions used with connector=superset accept both forms.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable; time-bound SQL literals change from a space separator to ISO 8601.

TESTING INSTRUCTIONS

pytest tests/unit_tests/db_engine_specs/test_dynamodb.py passes 13 tests after merging master, including its column-type and ORDER BY tests. Pre-commit passes. The new test checks the comparison against ISO 8601 strings directly.

I also checked it against DynamoDB Local 3.3.1 with PyDynamoDB and connector=superset: a raw chart and a COUNT-by-group chart over a 04:00–06:00 range returned the matching rows, where they previously returned none.

ADDITIONAL INFORMATION

• Changes UI (time-filtered results on DynamoDB)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.57%. Comparing base (92b0cf8) to head (d1299ac).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #44702       +/-   ##
===========================================
+ Coverage   57.25%   81.57%   +24.31%     
===========================================
  Files        2977     2977               
  Lines      180676   180659       -17     
  Branches    41786    41773       -13     
===========================================
+ Hits       103450   147367    +43917     
+ Misses      76213    30570    -45643     
- Partials     1013     2722     +1709     

Flag	Coverage Δ
hive	36.69% <0.00%> (ø)
mysql	55.95% <0.00%> (?)
postgres	55.95% <0.00%> (?)
presto	38.61% <0.00%> (ø)
python	85.71% <100.00%> (+47.05%)	⬆️
sqlite	55.68% <0.00%> (?)
unit	78.49% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bito-code-review]

The flagged issue is correct. Using timespec="seconds" truncates fractional seconds, which can lead to incorrect boundary comparisons when dealing with timestamps that include milliseconds. To resolve this, you should include the milliseconds in the ISO format to ensure accurate range filtering.

To fix this, update the timespec argument to "milliseconds" (or "auto" if you want to include them only when present) in superset/db_engine_specs/dynamodb.py:

            return f"'{dttm.isoformat(timespec="milliseconds")}'"

I have checked the available PR comments, and there are no other comments to address. Would you like me to proceed with any other changes?

superset/db_engine_specs/dynamodb.py

return f"'{dttm.isoformat(timespec="milliseconds")}'"

[bito-code-review]

Code Review Agent Run #40f781

Actionable Suggestions - 0

Additional Suggestions - 2

• tests/unit_tests/db_engine_specs/test_dynamodb.py - 2

  • Naive datetime in test · Line 66-66
    `spec.convert_dttm("text", datetime(2019, 1, 2, 4, 0, 0, 500000))` passes a naive datetime, so `dttm.isoformat()` yields "2019-01-02T04:00:00.500000" with no offset. If `convert_dttm` is ever called with an aware datetime (as request-time `dttm` values can be), the emitted literal would carry an offset suffix this test never exercises.
  • Test gap: upper bound · Line 58-58
    The compound assertion only exercises the exclusive upper bound: `high` ("2019-01-02T06:00:00") equals `stored[2]`, so `low <= value < high` filters it out. The inclusive `<=` upper-bound case that Superset's time-range filters emit is never asserted. Adding a second list with `low <= value <= high` would cover it.

Review Details

• Files reviewed - 2 · Commit Range: e6f4c3c..81c354d

  • superset/db_engine_specs/dynamodb.py
  • tests/unit_tests/db_engine_specs/test_dynamodb.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[rusackas]

LGTM. Good catch on the space-vs-T sort ordering, and codeant's fractional-seconds follow-up was real and got fixed properly, isoformat() keeping microseconds when set means whole-second bounds still sort correctly against sub-second stored values. Nice test on the boundary cases. Approving.

[EnxDev]

The ISO 8601 fix is right for tables that store T-separated timestamps, and the fractional-seconds follow-up holds up.

My main question is inline: tables that store space-separated strings get worse with this change, not just on sub-day ranges. The branch also conflicts with #44711, which appended tests to the same test file, so it needs a rebase.

[aminghadersohi]

Addressed the additional timezone-test suggestion in d1299ac. Regarding “Test gap: upper bound”: dataset time filters in superset/models/helpers.py use >= for the start and < for the end, so the existing exclusive-upper-bound assertion matches the production contract; an inclusive-end assertion would test a different contract. Fractional seconds remain covered by the fix in 81c354d.

[aminghadersohi]

@rusackas merged master without rewriting history and addressed the timestamp-format migration documentation and timezone coverage since your approval (d1299ac). All 13 DynamoDB tests and pre-commit pass.

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	d1299ac
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6abc878b1672980008d46cb7
😎 Deploy Preview	https://deploy-preview-44702--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[bito-code-review]

Bito Automatic Review Skipped – PR Already Merged

Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.
No action is needed if you didn't intend to review it. To get a review, you can type /review in a comment and save it