Uh oh!
There was an error while loading. Please reload this page.
fix(api): replace COALESCE with index-friendly OR conditions in datetime range filters - #66696
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
hkc-8010
commented
May 11, 2026
The failed check is
Failing job: This looks like transient infra/docs publish fallout rather than a regression from this PR. |
hkc-8010
commented
May 13, 2026
Code reviewFound 2 issues:
airflow/airflow-core/src/airflow/api_fastapi/common/parameters.py Lines 907 to 916 in 315d94d
airflow/airflow-core/src/airflow/api_fastapi/common/parameters.py Lines 938 to 945 in 315d94d 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
…ime range filters Adds NullableDatetimeRangeFilter, a RangeFilter subclass for start_date/end_date columns that emits OR predicates instead of COALESCE(col, now()), allowing PostgreSQL to use btree indexes on those columns. Two bugs fixed versus the original implementation in PR apache#66696: - Lower bounds now use or_(col >= x, col.is_(None)) without a now() guard, so future-scheduled tasks (NULL start_date) are never incorrectly excluded. - The factory dispatches on (attribute_name or filter_name), so aliased callers like datetime_range_filter_factory("dag_run_end_date", DagRun, "end_date") also receive NullableDatetimeRangeFilter rather than a plain RangeFilter.
315d94d to
2ff659cComparehkc-8010
commented
May 13, 2026
Both issues from the code review above have been addressed in the latest push (commit 2ff659c): Issue 1 — lower-bound Lower bounds now use Issue 2 — factory dispatch fixed for aliased filter names The dispatch condition changed from |
…bute_name
datetime_range_filter_factory("dag_run_start_date", DagRun, "start_date")
passes attribute_name="start_date", so the guard
if (attribute_name or filter_name) in ("start_date", "end_date"):
resolved to "start_date" and incorrectly returned NullableDatetimeRangeFilter
for the dag_run_start/end_date filters in the DAGs route. Those columns are
reached via an outer join; NULL means "no run", not "currently running", so
the OR (col IS NULL) branch inflated total_entries counts.
The original COALESCE guard checked filter_name only, so "dag_run_start_date"
was excluded. Revert to filter_name to preserve those semantics — only
callers with filter_name="start_date" or "end_date" (task instances, dag_run,
job routes) get NullableDatetimeRangeFilter.
Fixes TestGetDags::test_get_dags failures for query_params 13/14/17/21/23.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>The dag_run_start_date and dag_run_end_date filters in the DAGs route use an outer join, so NULL means "the DAG has no runs" — not "currently running". They must return a plain RangeFilter, not NullableDatetimeRangeFilter. Replace the two tests that incorrectly expected NullableDatetimeRangeFilter for aliased callers with tests that assert plain RangeFilter is returned. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
potiuk
commented
May 18, 2026
@hkc-8010 — There is 1 unresolved review thread on this PR from @ashb, and you have engaged with each one (post-review commits and/or in-thread replies). Could you confirm whether you believe the feedback is fully addressed and the PR is ready for maintainer review confirmation? If yes, reply here (a short "yes / ready" is fine) and an Apache Airflow maintainer will pick the PR up from the review queue on the next sweep. If you are still working on a thread, please reply with what is outstanding so the threads stay unresolved on purpose. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
hkc-8010
commented
May 18, 2026
The PR is ready for review. |
Backport successfully created: v3-2-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
…in API queries (apache#66696) * fix(api): replace COALESCE with index-friendly OR conditions in datetime range filters Adds NullableDatetimeRangeFilter, a RangeFilter subclass for start_date/end_date columns that emits OR predicates instead of COALESCE(col, now()), allowing PostgreSQL to use btree indexes on those columns. Two bugs fixed versus the original implementation in PR apache#66696: - Lower bounds now use or_(col >= x, col.is_(None)) without a now() guard, so future-scheduled tasks (NULL start_date) are never incorrectly excluded. - The factory dispatches on (attribute_name or filter_name), so aliased callers like datetime_range_filter_factory("dag_run_end_date", DagRun, "end_date") also receive NullableDatetimeRangeFilter rather than a plain RangeFilter. * fix(api): scope NullableDatetimeRangeFilter to filter_name, not attribute_name datetime_range_filter_factory("dag_run_start_date", DagRun, "start_date") passes attribute_name="start_date", so the guard if (attribute_name or filter_name) in ("start_date", "end_date"): resolved to "start_date" and incorrectly returned NullableDatetimeRangeFilter for the dag_run_start/end_date filters in the DAGs route. Those columns are reached via an outer join; NULL means "no run", not "currently running", so the OR (col IS NULL) branch inflated total_entries counts. The original COALESCE guard checked filter_name only, so "dag_run_start_date" was excluded. Revert to filter_name to preserve those semantics — only callers with filter_name="start_date" or "end_date" (task instances, dag_run, job routes) get NullableDatetimeRangeFilter. Fixes TestGetDags::test_get_dags failures for query_params 13/14/17/21/23. * fix(tests): correct test_aliased_*_returns_nullable_filter assertions The dag_run_start_date and dag_run_end_date filters in the DAGs route use an outer join, so NULL means "the DAG has no runs" — not "currently running". They must return a plain RangeFilter, not NullableDatetimeRangeFilter. Replace the two tests that incorrectly expected NullableDatetimeRangeFilter for aliased callers with tests that assert plain RangeFilter is returned. (cherry picked from commit 37667f1) Co-authored-by: Hemkumar Chheda <95332229+hkc-8010@users.noreply.github.com>
…in API queries (#66696) (#67102) * fix(api): replace COALESCE with index-friendly OR conditions in datetime range filters Adds NullableDatetimeRangeFilter, a RangeFilter subclass for start_date/end_date columns that emits OR predicates instead of COALESCE(col, now()), allowing PostgreSQL to use btree indexes on those columns. Two bugs fixed versus the original implementation in PR #66696: - Lower bounds now use or_(col >= x, col.is_(None)) without a now() guard, so future-scheduled tasks (NULL start_date) are never incorrectly excluded. - The factory dispatches on (attribute_name or filter_name), so aliased callers like datetime_range_filter_factory("dag_run_end_date", DagRun, "end_date") also receive NullableDatetimeRangeFilter rather than a plain RangeFilter. * fix(api): scope NullableDatetimeRangeFilter to filter_name, not attribute_name datetime_range_filter_factory("dag_run_start_date", DagRun, "start_date") passes attribute_name="start_date", so the guard if (attribute_name or filter_name) in ("start_date", "end_date"): resolved to "start_date" and incorrectly returned NullableDatetimeRangeFilter for the dag_run_start/end_date filters in the DAGs route. Those columns are reached via an outer join; NULL means "no run", not "currently running", so the OR (col IS NULL) branch inflated total_entries counts. The original COALESCE guard checked filter_name only, so "dag_run_start_date" was excluded. Revert to filter_name to preserve those semantics — only callers with filter_name="start_date" or "end_date" (task instances, dag_run, job routes) get NullableDatetimeRangeFilter. Fixes TestGetDags::test_get_dags failures for query_params 13/14/17/21/23. * fix(tests): correct test_aliased_*_returns_nullable_filter assertions The dag_run_start_date and dag_run_end_date filters in the DAGs route use an outer join, so NULL means "the DAG has no runs" — not "currently running". They must return a plain RangeFilter, not NullableDatetimeRangeFilter. Replace the two tests that incorrectly expected NullableDatetimeRangeFilter for aliased callers with tests that assert plain RangeFilter is returned. (cherry picked from commit 37667f1) Co-authored-by: Hemkumar Chheda <95332229+hkc-8010@users.noreply.github.com>
…in API queries (#66696) (#67102) * fix(api): replace COALESCE with index-friendly OR conditions in datetime range filters Adds NullableDatetimeRangeFilter, a RangeFilter subclass for start_date/end_date columns that emits OR predicates instead of COALESCE(col, now()), allowing PostgreSQL to use btree indexes on those columns. Two bugs fixed versus the original implementation in PR #66696: - Lower bounds now use or_(col >= x, col.is_(None)) without a now() guard, so future-scheduled tasks (NULL start_date) are never incorrectly excluded. - The factory dispatches on (attribute_name or filter_name), so aliased callers like datetime_range_filter_factory("dag_run_end_date", DagRun, "end_date") also receive NullableDatetimeRangeFilter rather than a plain RangeFilter. * fix(api): scope NullableDatetimeRangeFilter to filter_name, not attribute_name datetime_range_filter_factory("dag_run_start_date", DagRun, "start_date") passes attribute_name="start_date", so the guard if (attribute_name or filter_name) in ("start_date", "end_date"): resolved to "start_date" and incorrectly returned NullableDatetimeRangeFilter for the dag_run_start/end_date filters in the DAGs route. Those columns are reached via an outer join; NULL means "no run", not "currently running", so the OR (col IS NULL) branch inflated total_entries counts. The original COALESCE guard checked filter_name only, so "dag_run_start_date" was excluded. Revert to filter_name to preserve those semantics — only callers with filter_name="start_date" or "end_date" (task instances, dag_run, job routes) get NullableDatetimeRangeFilter. Fixes TestGetDags::test_get_dags failures for query_params 13/14/17/21/23. * fix(tests): correct test_aliased_*_returns_nullable_filter assertions The dag_run_start_date and dag_run_end_date filters in the DAGs route use an outer join, so NULL means "the DAG has no runs" — not "currently running". They must return a plain RangeFilter, not NullableDatetimeRangeFilter. Replace the two tests that incorrectly expected NullableDatetimeRangeFilter for aliased callers with tests that assert plain RangeFilter is returned. (cherry picked from commit 37667f1) Co-authored-by: Hemkumar Chheda <95332229+hkc-8010@users.noreply.github.com>
…in API queries (#66696) (#67102) * fix(api): replace COALESCE with index-friendly OR conditions in datetime range filters Adds NullableDatetimeRangeFilter, a RangeFilter subclass for start_date/end_date columns that emits OR predicates instead of COALESCE(col, now()), allowing PostgreSQL to use btree indexes on those columns. Two bugs fixed versus the original implementation in PR #66696: - Lower bounds now use or_(col >= x, col.is_(None)) without a now() guard, so future-scheduled tasks (NULL start_date) are never incorrectly excluded. - The factory dispatches on (attribute_name or filter_name), so aliased callers like datetime_range_filter_factory("dag_run_end_date", DagRun, "end_date") also receive NullableDatetimeRangeFilter rather than a plain RangeFilter. * fix(api): scope NullableDatetimeRangeFilter to filter_name, not attribute_name datetime_range_filter_factory("dag_run_start_date", DagRun, "start_date") passes attribute_name="start_date", so the guard if (attribute_name or filter_name) in ("start_date", "end_date"): resolved to "start_date" and incorrectly returned NullableDatetimeRangeFilter for the dag_run_start/end_date filters in the DAGs route. Those columns are reached via an outer join; NULL means "no run", not "currently running", so the OR (col IS NULL) branch inflated total_entries counts. The original COALESCE guard checked filter_name only, so "dag_run_start_date" was excluded. Revert to filter_name to preserve those semantics — only callers with filter_name="start_date" or "end_date" (task instances, dag_run, job routes) get NullableDatetimeRangeFilter. Fixes TestGetDags::test_get_dags failures for query_params 13/14/17/21/23. * fix(tests): correct test_aliased_*_returns_nullable_filter assertions The dag_run_start_date and dag_run_end_date filters in the DAGs route use an outer join, so NULL means "the DAG has no runs" — not "currently running". They must return a plain RangeFilter, not NullableDatetimeRangeFilter. Replace the two tests that incorrectly expected NullableDatetimeRangeFilter for aliased callers with tests that assert plain RangeFilter is returned. (cherry picked from commit 37667f1) Co-authored-by: Hemkumar Chheda <95332229+hkc-8010@users.noreply.github.com>
…in API queries (#66696) (#67102) * fix(api): replace COALESCE with index-friendly OR conditions in datetime range filters Adds NullableDatetimeRangeFilter, a RangeFilter subclass for start_date/end_date columns that emits OR predicates instead of COALESCE(col, now()), allowing PostgreSQL to use btree indexes on those columns. Two bugs fixed versus the original implementation in PR #66696: - Lower bounds now use or_(col >= x, col.is_(None)) without a now() guard, so future-scheduled tasks (NULL start_date) are never incorrectly excluded. - The factory dispatches on (attribute_name or filter_name), so aliased callers like datetime_range_filter_factory("dag_run_end_date", DagRun, "end_date") also receive NullableDatetimeRangeFilter rather than a plain RangeFilter. * fix(api): scope NullableDatetimeRangeFilter to filter_name, not attribute_name datetime_range_filter_factory("dag_run_start_date", DagRun, "start_date") passes attribute_name="start_date", so the guard if (attribute_name or filter_name) in ("start_date", "end_date"): resolved to "start_date" and incorrectly returned NullableDatetimeRangeFilter for the dag_run_start/end_date filters in the DAGs route. Those columns are reached via an outer join; NULL means "no run", not "currently running", so the OR (col IS NULL) branch inflated total_entries counts. The original COALESCE guard checked filter_name only, so "dag_run_start_date" was excluded. Revert to filter_name to preserve those semantics — only callers with filter_name="start_date" or "end_date" (task instances, dag_run, job routes) get NullableDatetimeRangeFilter. Fixes TestGetDags::test_get_dags failures for query_params 13/14/17/21/23. * fix(tests): correct test_aliased_*_returns_nullable_filter assertions The dag_run_start_date and dag_run_end_date filters in the DAGs route use an outer join, so NULL means "the DAG has no runs" — not "currently running". They must return a plain RangeFilter, not NullableDatetimeRangeFilter. Replace the two tests that incorrectly expected NullableDatetimeRangeFilter for aliased callers with tests that assert plain RangeFilter is returned. (cherry picked from commit 37667f1) Co-authored-by: Hemkumar Chheda <95332229+hkc-8010@users.noreply.github.com>
Summary
`COALESCE(column, now())` to treat currently-running tasks (NULL date) as "now".
This form prevents PostgreSQL from using btree indexes, causing parallel sequential
scans across the full `task_instance` table even for narrow date windows.
`(col >= X) OR (col IS NULL AND now() >= X)`
PostgreSQL can use a btree index on each OR branch via BitmapOr. Running tasks
(NULL `end_date` / `start_date`) continue to match date-range queries correctly.
`datetime_range_filter_factory` returns this subclass for `start_date` / `end_date`
and a plain `RangeFilter` for all other filter names.
Performance
Measured against a 4.6M-row `task_instance` table with an `end_date` btree index:
7.4x reduction in query cost.
Note: vanilla Airflow does not ship an `end_date` index by default. The OR form is
the correct architectural fix regardless. Any deployment that adds an `end_date` index
(via a migration) will immediately benefit. A follow-up migration to add the index is
recommended but is outside the scope of this PR.
Changes
`NullableDatetimeRangeFilter`; update `datetime_range_filter_factory`
`TestDatetimeRangeFilterFactory` (7 tests covering type dispatch, SQL shape,
no-COALESCE assertion, and NULL-branch presence)
PR Checklist
closes: #66335