[FrancescoCastaldi]

SUMMARY

Fixes #45459.

When a temporal filter carries an unparseable time range (such as "a : b" or inverted date bounds like "2024-02-01 : 2024-01-01"), get_since_until_from_time_range raises TimeRangeParseFailError (subclass of marshmallow ValidationError) or ValueError.

When such an invalid time range is present in a secondary temporal filter or during date-range timeshift processing:

• In get_sqla_query, the parse failure was uncaught and escaped the query validation handlers, reaching the global Flask error handler and returning HTTP 500 instead of HTTP 400.
• In processing_time_offsets, relative and date-range offsets experienced similar uncaught parse errors.

This change:

1. Catches (TimeRangeParseFailError, TimeRangeAmbiguousError, ValidationError, ValueError) across get_sqla_query and processing_time_offsets.
2. Converts the error into QueryObjectValidationError naming the invalid column, enabling QueryContextProcessor to return an informative 400 Bad Request.
3. Adds comprehensive unit tests in tests/unit_tests/models/helpers_test.py.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A (Backend API error handling)

TESTING INSTRUCTIONS

Run unit tests:

pytest tests/unit_tests/models/helpers_test.py

ADDITIONAL INFORMATION

• Has associated issue: Fixes A malformed time range in a second temporal filter returns 500 instead of 400 #45459
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
• Introduces new feature or API
• Removes existing feature or API

[codeant-ai-for-open-source]

  CodeAnt PR Risk

🟢  Low Risk

• The PR appears safe to merge; no actionable issues were identified in the review.

Assessed commit: 089c8d37a43f

[SEPURI-SAI-KRISHNA]

Heads up, #45461 has been open for this since Oct 19 and is linked on the
issue. It covers the same four parse sites:
get_since_until_from_query_object in processing_time_offsets, the
date-range offset parse, the temporal filter rewrite in that loop, and the one
in get_sqla_query.

The only difference I can see is where the date-range offset is caught. You
widen the inner except beside the call, mine widens the outer one that
encloses it. Same effect, since that call is the only parse inside the outer
try.

No preference on which lands, just flagging so we don't both carry it. The
reversed-range test asked for on mine went in Oct 25, so it isn't blocked on
my side.

[codecov]

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.63%. Comparing base (c7f17e0) to head (089c8d3).
⚠️ Report is 52 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/models/helpers.py	91.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #45822      +/-   ##
==========================================
+ Coverage   84.57%   84.63%   +0.06%     
==========================================
  Files        3069     3068       -1     
  Lines      205228   205617     +389     
  Branches    48222    48317      +95     
==========================================
+ Hits       173571   174033     +462     
+ Misses      28353    28292      -61     
+ Partials     3304     3292      -12     

Flag	Coverage Δ
hive	34.51% <12.50%> (-0.06%)	⬇️
mysql	52.58% <75.00%> (+0.07%)	⬆️
postgres	52.58% <75.00%> (+0.07%)	⬆️
presto	36.23% <37.50%> (-0.06%)	⬇️
python	87.76% <91.66%> (+0.10%)	⬆️
sqlite	52.35% <75.00%> (+0.07%)	⬆️
unit	82.49% <75.00%> (+0.17%)	⬆️

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.

[FrancescoCastaldi]

Executive Summary & Review Feedback Resolution

All feedback from @sadpandajoe on PR #45822 has been addressed, verified, and pushed to origin in commit e74e08b55c.

1. Addressed Review Items & Foresight Anticipation

• QueryObject Filter Parameter Fix: corrected tests/unit_tests/models/helpers_test.py to use filters= instead of filter=, preventing clauses from being swallowed by **kwargs.
• Relative Offset Filter-Rewrite Verification: structured test_processing_time_offsets_malformed_relative_filter_raises_validation_error with a valid outer time range and non-datetime index series alongside a malformed second temporal filter (col="other_ds", val="a : b") and relative offset ("1 day ago"). Verified that QueryObjectValidationError names other_ds and chains TimeRangeParseFailError on __cause__.
• Date Range Timeshifts Path (DATE_RANGE_TIMESHIFTS_ENABLED): added test_processing_time_offsets_malformed_date_range_offset_raises_validation_error covering inverted date ranges like "2024-02-01 : 2024-01-01" when date range timeshifts are enabled, verifying the exception chaining.
• Outer Time Range Validation: added test_processing_time_offsets_malformed_outer_time_range_raises_validation_error for outer temporal range parsing failures.
• get_sqla_query Chaining: added assertions confirming TimeRangeParseFailError is chained on __cause__.
• Elimination of Translation Template Drift: in superset/models/helpers.py, sanitized QueryObjectValidationError message strings without _(), resolving the pot template drift detected by scripts/translations/check_pot_drift.py and babel-extract.

2. Modified Files & Test Coverage

• superset/models/helpers.py: updated validation exception strings and exception chaining.
• tests/unit_tests/models/helpers_test.py: added 3 comprehensive unit test suites covering outer range, relative offset filter-rewrite, and date range timeshifts.

3. Evidence Gate & Quality Checks

• Line length audit: 100% compliant with Ruff and Black strict 88-column limit.
• Python syntax compilation: passed with zero errors.
• Translation check (babel-extract): "Check messages.pot matches a fresh extraction" passed with success.

@sadpandajoe, all requested test improvements and CI fixes are in place. The PR is ready for your re-review whenever you have a moment, thank you!

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	e74e08b
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6ae3bec9c83ca921862dedc7
😎 Deploy Preview	https://deploy-preview-45822--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.

[sadpandajoe]

A secondary temporal filter like "2024-01-01T00:00:00Z : 2024-01-02" still returns a 500: get_since_until compares an offset-aware _since with a naive _until (_since > _until) and raises TypeError, which isn't in this tuple, so it escapes the new handler. The same gap applies to the other guards added here. Should TypeError be caught too, or is mixed-timezone input intentionally out of scope for this fix?

[FrancescoCastaldi]

In commit 089c8d37a4, we addressed this by adding TypeError to the caught exception tuple across all three get_since_until_from_time_range call sites in superset/models/helpers.py (lines 3089, 3227, and 7027). When mixed offset-naive and offset-aware datetimes like "2024-01-01T00:00:00Z : 2024-01-02" are passed, get_since_until raises a TypeError during timestamp comparison (_since > _until). This is now properly intercepted and converted into a QueryObjectValidationError, consistently returning 400 instead of 500.

Additionally, we formatted the exception statement in superset/models/helpers.py to satisfy ruff-format (resolving the pre-commit (current) CI failure) and added a dedicated regression unit test in tests/unit_tests/models/helpers_test.py verifying mixed-timezone temporal ranges.

[FrancescoCastaldi]

Review Follow-Up & CI Remediation

All review comments and CI checks have been addressed in commit 089c8d37a4:

1. Mixed Timezone Comparison Fix:

   • Added TypeError to the caught exception tuple across all three get_since_until_from_time_range call sites in superset/models/helpers.py.
   • When mixed naive and offset-aware datetimes like "2024-01-01T00:00:00Z : 2024-01-02" are parsed, get_since_until raises TypeError: can't compare offset-naive and offset-aware datetimes. This is now safely intercepted and re-raised as QueryObjectValidationError, returning a 400 status code instead of escaping as a 500 error.
   • Added dedicated regression unit test in tests/unit_tests/models/helpers_test.py.

2. Pre-commit Linter / Formatting Fix:

   • Formatted superset/models/helpers.py with ruff-format (verified clean and within the 88-column limit) to resolve the pre-commit (current) CI status check.

@sadpandajoe, this is ready for your re-review whenever you have a chance. Thank you!