kaxil commented on issue #65125:
URL: https://github.com/apache/airflow/issues/65125#issuecomment-4239936468

   ## Issue Triage
   
   I reviewed the code at 
[`airflow-core/src/airflow/models/dagrun.py:863-870`](https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/models/dagrun.py#L863-L870)
 and this is a **valid bug**.
   
   ### The NULL ordering issue is real
   
   The query uses `.order_by(DagRun.logical_date.desc())` but `logical_date` is 
nullable (`Mapped[datetime | None]`). In Airflow 3, manual runs can have 
`logical_date=None`. NULL sort behavior is database-dependent:
   
   - **PostgreSQL**: NULLs sort first in DESC (they appear at the top)
   - **MySQL**: NULLs sort first in DESC  
   - **SQLite**: NULLs sort first in DESC
   
   This means NULL-logical-date manual runs will appear at the top of the 
result set, pushing out legitimate scheduled runs from the "last N" window.
   
   ### Suggested fix for the ordering
   
   Replace `DagRun.logical_date.desc()` with `DagRun.run_after.desc()` -- 
`run_after` is non-nullable (`nullable=False`), always populated, and gives a 
deterministic, stable ordering that represents when each run was actually 
created/scheduled. Alternatively, `DagRun.id.desc()` works too since it's an 
auto-incrementing PK.
   
   ### On run_type filtering
   
   The suggestion to filter by `run_type` (e.g., only count scheduled runs) is 
more of a design/enhancement discussion. There are valid arguments for both 
sides:
   - **For filtering**: Manual test runs shouldn't pause production schedules
   - **Against filtering**: If someone triggers 3 manual runs and they all fail 
due to a real problem, auto-pausing is a reasonable safety behavior
   
   This part should probably go through a discussion with committers since it 
changes the feature's semantics. A separate issue or PR discussion would be 
appropriate.
   
   ### Origin
   
   This isn't a regression from a recent commit -- the function has been 
unchanged since PR #36935 originally introduced it. It became a problem when 
`logical_date` was made nullable in the Airflow 3 model changes.
   
   PR #47301 fixed a similar NULL-logical-date issue in 
`get_previous_scheduled_dagrun` but didn't touch this function.


-- 
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]

Reply via email to