manipatnam opened a new pull request, #67551:
URL: https://github.com/apache/airflow/pull/67551
`order_by=rendered_map_index` on the `listMapped` and `listTaskInstances`
endpoints had two bugs:
1. **Retried TIs silently pushed off the first page** — when
`_rendered_map_index` was NULL for all rows (no `map_index_template`), the sort
fell through to the UUID tiebreaker. Retried TIs receive a new UUID at retry
time (a larger, later timestamp), so they sorted to the end of the result set
and fell outside `LIMIT N`. Already partially addressed by #66008, which added
the SQL `.expression` fallback `CAST(map_index AS String)`.
2. **Lexicographic ordering for 10+ mapped instances** — the `CAST(map_index
AS String)` fallback produces string comparison: `"10"` sorts before `"2"`,
`"20"` before `"3"`, etc. A Dag with 12 mapped tasks returned them as `0, 1,
10, 11, 2, 3 …` instead of `0, 1, 2, 3 … 10, 11`.
closes: #67451
## Root cause
`SortParam` resolved `rendered_map_index` to a single column for `ORDER BY`.
With a nullable primary column and a string-cast fallback, there was no stable
integer secondary key, so UUID ordering determined page membership for tasks
without `map_index_template`.
## Fix
Extend `SortParam.to_replace` to accept `list[Column]` as a **compound
sort**: a single user-facing sort key expands into multiple `ORDER BY` columns.
Both `listMapped` and `listTaskInstances` now map `rendered_map_index` to
`[TI._rendered_map_index, TI.map_index]`,producing:
```sql
ORDER BY _rendered_map_index ASC, -- NULL when no map_index_template
map_index ASC, -- integer, always present
id ASC -- UUID, absolute tiebreaker
```
| Scenario | Result |
|---|---|
| No `map_index_template` (common case) | All `_rendered_map_index` NULL →
tie on key 1 → sorted by integer `map_index` → `0, 1, 2, 10, 11 …` |
| With `map_index_template` | Sorted alphabetically by custom label, then
`map_index` for ties |
| Retried TI (newer UUID, same `map_index`) | UUID is last tiebreaker only —
no longer pushed off the first page |
## Changes
**`airflow/api_fastapi/common/parameters.py`**
- `SortParam.__init__` type annotation extended: `dict[str, str | Column |
list[Column]] | None`
- `SortParam._resolve()` — when a `to_replace` value is a `list`, each
column is expanded into its own sort entry using the column's ORM
`.key` as `attr_name`, so `row_value()` resolves it via `getattr` without
any additional lookup.
- `SortParam.row_value()` — guard updated to skip list-form values (they are
already expanded in `_resolve()`).
**`airflow/api_fastapi/core_api/routes/public/task_instances.py`**
- `listMapped` and `listTaskInstances` `SortParam` both gain
`"rendered_map_index": [TI._rendered_map_index, TI.map_index]` in their
`to_replace` dicts.
## Tests
- Updated the two `rendered_map_index` / `-rendered_map_index` cases in the
existing `test_mapped_instances_order` parametrize to expect numeric order
(`list(range(100))` / `list(range(109, 9, -1))`) instead of the previous
lexicographic expectations (`sorted(range(110), key=str)`).
- **New** `test_rendered_map_index_order_with_template`: verifies that when
all TIs have a custom `_rendered_map_index` label the sort is alphabetical by
label.
- **New** `test_rendered_map_index_order_retried_tis_not_displaced`:
simulates retries by assigning newer UUIDs to some TIs and verifies all TIs
still appear on the first page in `map_index` order.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (claude-sonnet-4-6)
Generated-by: Claude Code (claude-sonnet-4-6) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
