Skip to content

fixed sort order for mapped task instances#67551

Open
manipatnam wants to merge 5 commits into
apache:mainfrom
manipatnam:fix-sort-mapped-tasks-order
Open

fixed sort order for mapped task instances#67551
manipatnam wants to merge 5 commits into
apache:mainfrom
manipatnam:fix-sort-mapped-tasks-order

Conversation

@manipatnam
Copy link
Copy Markdown
Contributor

@manipatnam manipatnam commented May 26, 2026

order_by=rendered_map_index on the listMapped and listTaskInstances endpoints had a bug:

  1. 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:

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_index0, 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?
  • Yes — Claude Code (claude-sonnet-4-6)

Generated-by: Claude Code (claude-sonnet-4-6) following the guidelines

Copy link
Copy Markdown
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have screenshot of before/after to highlight the behavior.

There is also the get_task_instances endpoint that specifies the rendered_map_indexes in the order param, that should probably be also updated.

I think this fixes the part where rendered_map_index is None, but that's actually almost never the case. Unless the dynamic tasks mapping isn't expended yet, rendred_map_index will fallback to the map_index.

Comment thread airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_task_instances.py Outdated
Comment thread airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_task_instances.py Outdated
@manipatnam
Copy link
Copy Markdown
Contributor Author

Tested using the following DAG:

from datetime import datetime

from airflow.decorators import dag, task


@dag(
    dag_id="dynamic_mapped_dag",
    start_date=datetime(2026, 1, 1),
    schedule=None,
    catchup=False,
    tags=["dynamic", "mapped"],
)
def dynamic_mapped_dag():
    @task
    def generate_indexes() -> list[int]:
        return list(range(126))

    @task
    def process(index: int) -> int:
        print(f"Processing map_index={index}")
        return index

    @task(map_index_template="{{ user }}")
    def process_labeled(user: str) -> str:
        print(f"Processing user={user}")
        return user

    process.expand(index=generate_indexes())
    process_labeled.expand(user=["zebra", "apple", "mango", "cherry", "banana"])


dynamic_mapped_dag()

Task: process

Before this PR (including #66008 fix)

Default view

Default view

When sorted ascending

Sorted ascending

When sorted descending

Sorted descending


Using the code in this PR (also includes #66008 fix)

Default behaviour when no filter is applied

Default behaviour

When sorted ascending

Sorted ascending

When sorted descending

Sorted descending

Default behaviour when map_index 124 and 123 are retried

Retry behaviour


Task: process_labeled (Nothing changed)

Before this PR (including #66008 fix)

Default view

Default view

When sorted ascending

Sorted ascending

When sorted descending

Sorted descending

Default view when map_index cherry and mango are retried

Retry behaviour


Using the code in this PR (also includes #66008 fix)

Default behaviour when no filter is applied

Default behaviour

When sorted ascending

Sorted ascending

When sorted descending

Sorted descending

Default behaviour when map_index cherry and mango are retried

Retry behaviour

@manipatnam manipatnam requested a review from pierrejeambrun May 26, 2026 15:12
Comment thread airflow-core/src/airflow/api_fastapi/common/parameters.py Outdated
pierrejeambrun
pierrejeambrun previously approved these changes May 27, 2026
Copy link
Copy Markdown
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks.

CI isn't happy some tests need adjustment.

One nit / question and good to go I believe.

@pierrejeambrun pierrejeambrun added the backport-to-v3-2-test Mark PR with this label to backport to v3-2-test branch label May 27, 2026
@pierrejeambrun pierrejeambrun added this to the Airflow 3.2.3 milestone May 27, 2026
@pierrejeambrun pierrejeambrun added the full tests needed We need to run full set of tests for this PR to merge label May 27, 2026
@pierrejeambrun pierrejeambrun dismissed their stale review May 27, 2026 11:45

Error see comment

@pierrejeambrun
Copy link
Copy Markdown
Member

Pagination isn't working anymore:

Screen.Recording.2026-05-27.at.13.46.50.mov

AI Assistant added 3 commits May 27, 2026 19:19
…iption

When `rendered_map_index` (and other keys like `run_after`, `logical_date`,
`data_interval_start`, `data_interval_end`) are present in both `allowed_attrs`
and `to_replace`, `dynamic_depends` was appending them twice to `all_attrs`,
causing the generated `order_by` parameter description to list those attributes
twice. This caused the `generate-openapi-spec` static check to fail.

Fix: deduplicate `to_replace_attrs` by excluding keys already in `allowed_attrs`
before building `all_attrs`. Update the committed OpenAPI spec to match.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API backport-to-v3-2-test Mark PR with this label to backport to v3-2-test branch full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

order_by=rendered_map_index lexicographic sort on map_index when _rendered_map_index is None

2 participants