hkc-8010 commented on PR #66696: URL: https://github.com/apache/airflow/pull/66696#issuecomment-4443217234
### Code review Found 2 issues: 1. Lower-bound NULL branch still includes `func.now() >= x`, which breaks filter semantics for future-scheduled tasks. For a lower bound, a task with `NULL start_date` (not yet started) should unconditionally pass — its eventual value will always be >= any past bound. With `now() >= x`, if `x` is in the future, `now() >= x` is `FALSE`, so future-scheduled tasks are incorrectly excluded. Reviewer `ashb` explicitly called this out and suggested `or_(self.attribute.is_(None), self.attribute >= x)` for lower bounds. The author's reply acknowledges this, but the code and the test `test_lower_bound_calls_now_for_running_tasks` (which asserts `"now()"` still appears in lower-bound SQL) contradict the claimed fix. https://github.com/apache/airflow/blob/315d94d5027896259d30450b7c5a6712874d694e/airflow-core/src/airflow/api_fastapi/common/parameters.py#L907-L916 2. The factory dispatch `if filter_name in ("start_date", "end_date")` checks the API-facing filter name, not the resolved attribute name. Callers in `dags.py` pass aliased names (`"dag_run_start_date"`, `"dag_run_end_date"`) with `attribute_name="start_date"` / `"end_date"`, so they fall through to a plain `RangeFilter`. Since `DagRun.end_date` is `NULL` for running DAGs, these callers silently miss the index-friendly optimization and the correct NULL semantics. The test `test_attribute_name_alias_returns_plain_range_filter` asserts this as correct behavior, but that contradicts the PR's stated goal. The fix from a previous iteration of this PR (`attribute_name or filter_name`) would cover these callers. https://github.com/apache/airflow/blob/315d94d5027896259d30450b7c5a6712874d694e/airflow-core/src/airflow/api_fastapi/common/parameters.py#L938-L945 🤖 Generated with [Claude Code](https://claude.ai/code) <sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub> -- 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]
