manipatnam commented on code in PR #67551:
URL: https://github.com/apache/airflow/pull/67551#discussion_r3304805552
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -930,6 +933,80 @@ def test_rendered_map_index_filter(
assert body["total_entries"] == len(expected_map_indexes)
assert [ti["map_index"] for ti in body["task_instances"]] ==
expected_map_indexes
+ def test_rendered_map_index_order_with_template(self, test_client,
session, one_task_with_mapped_tis):
+ """Custom map_index_template labels must be sorted alphabetically."""
+ # one_task_with_mapped_tis creates 3 TIs: map_index 0, 1, 2.
+ labels = {0: "zebra", 1: "apple", 2: "mango"}
+ for map_index, label in labels.items():
+ ti = session.scalar(
+ select(TaskInstance).where(
+ TaskInstance.task_id == "task_2",
+ TaskInstance.map_index == map_index,
+ )
+ )
+ ti._rendered_map_index = label
+ session.commit()
+
+ response = test_client.get(
+
"/dags/mapped_tis/dagRuns/run_mapped_tis/taskInstances/task_2/listMapped",
+ params={"order_by": "rendered_map_index"},
+ )
+ assert response.status_code == 200
+ body = response.json()
+ # Alphabetical order: "apple" (1), "mango" (2), "zebra" (0).
+ assert [ti["map_index"] for ti in body["task_instances"]] == [1, 2, 0]
+ assert [ti["rendered_map_index"] for ti in body["task_instances"]] == [
+ "apple",
+ "mango",
+ "zebra",
+ ]
+
+ def test_rendered_map_index_order_retried_tis_not_displaced(self,
test_client, session, dag_maker):
+ """
+ Retried mapped TIs (newer UUID, same map_index) must appear in
map_index
+ order and not be pushed beyond the first page.
+
+ Before the fix, order_by=rendered_map_index fell through to a UUID
+ tiebreaker when _rendered_map_index was NULL. Retried TIs received a
+ newer UUID at retry time and therefore sorted to the end of the result
+ set, falling outside LIMIT N.
+ """
+ from sqlalchemy import update as sa_update
+
+ from airflow.models.taskinstance import uuid7
+
Review Comment:
Thanks for highlighting this - the original test description was misleading.
Since each TI has a unique map_index, the CAST(map_index AS String) fallback
always produces a unique value per TI, so UUID is never the effective
tiebreaker in practice. I've renamed the test to
test_rendered_map_index_order_stable_regardless_of_uuid_order with an updated
docstring.
The actual bug is purely lexicographic ordering — CAST(map_index AS String)
sorts "0","1","10","11","2"... instead of 0, 1, 2, ..., 10, 11. The compound
sort replaces that string fallback with the integer map_index column. I've also
added test_rendered_map_index_order_without_template_numeric which creates 12
TIs to directly cover the map_index > 9 case.
--
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]
