New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use COALESCE when ordering runs to handle NULL #26626
Conversation
Data interval columns are NULL for runs created before 2.3, but SQL's NULL-sorting logic would make those old runs always appear first. In a perfect world we'd want to sort by get_run_data_interval(), but that's not efficient, so instead the columns are coalesced into logical date, which is good enough in most cases.
dag_runs = query.order_by(*ordering, DagRun.id.desc()).limit(num_runs).all() | ||
dag_runs.reverse() | ||
|
||
dag_runs = wwwutils.sorted_dag_runs(query, ordering=dag.timetable.run_ordering, limit=num_runs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels more like it should be a method on DagRun class maybe? (That way this can be used by API or CLI too)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function in general includes some UI-specific sorting/slicing logic I don’t feel right making generic. The inner _get_run_ordering_expr
probably should, but I’d rather wait until when we actually reuse it somewhere (we don’t right now).
Data interval columns are NULL for runs created before 2.3, but SQL's NULL-sorting logic would make those old runs always appear first. In a perfect world we'd want to sort by get_run_data_interval(), but that's not efficient, so instead the columns are coalesced into logical date, which is good enough in most cases. (cherry picked from commit 22d52c0)
Data interval columns are NULL for runs created before 2.3, but SQL's NULL-sorting logic would make those old runs always appear first. In a perfect world we'd want to sort by get_run_data_interval(), but that's not efficient, so instead the columns are coalesced into logical date, which is good enough in most cases. (cherry picked from commit 22d52c0)
Data interval columns are NULL for runs created before 2.3, but SQL's NULL-sorting logic would make those old runs always appear first. In a perfect world we'd want to sort by get_run_data_interval(), but that's not efficient, so instead the columns are coalesced into logical date, which is good enough in most cases.
This should #26505, I think. Not sure if the UI side needs some additional work. cc @bbovenzi