sahilSENPAII opened a new pull request, #75015:
URL: https://github.com/apache/airflow/pull/75015
related: #69204, #72584
### Problem
`GET /execution/xcoms/{dag_id}/{run_id}/{task_id}/{key}/slice` returns wrong
rows, or fails, when one slice bound lies before the first mapped XCom once it
is resolved against the row count. With 3 mapped XComs `["f", "o", "b"]`:
| Slice | Python result | Query sent | SQLite | Postgres / MySQL |
|---|---|---|---|---|
| `seq[-2:10]` | `["o", "b"]` | `LIMIT 9 OFFSET -7` | `["f", "o", "b"]` |
negative offset rejected |
| `seq[1:-10:-1]` | `["o", "f"]` | `LIMIT 8 OFFSET -6` | `["b", "o", "f"]` |
negative offset rejected |
| `seq[2:-10:-1]` | `["b", "o", "f"]` | `LIMIT 9 OFFSET -6` | `["b", "o",
"f"]` (correct by chance) | negative offset rejected |
### Cause
`_get_sliced_query_or_empty` passes `low` to `Select.slice()` unchanged, and
SQLAlchemy compiles a negative `low` to a negative `OFFSET` with `LIMIT high -
low`. SQLite treats the negative offset as 0, so the inflated limit returns
extra rows. Postgres and MySQL reject the query. A negative `low` is possible
in two branches of `get_mapped_xcom_by_slice`: a non-negative `start` with a
negative `step` and a `stop` below `-len`, and a negative `start` with a
positive `step` and a `stop` above `len`. #72584 fixed the crossed-bounds case
(`high <= low`) in the same helper, but not this one.
### Fix
Clamp `low` to 0 in `_get_sliced_query_or_empty` before the existing `high
<= low` check. This matches how Python slicing clamps an out-of-range bound to
the start of the sequence. The other two call sites always pass a non-negative
`low`, so they are unchanged.
### Testing
- Added `-2:10`, `2:-10:-1` and `1:-10:-1` to the parametrized
`test_xcom_get_with_slice`, which compares the response with Python slicing.
- Without the fix, on SQLite: `-2:10` and `1:-10:-1` fail, and `2:-10:-1`
passes because the inflated limit happens to cover every row. Postgres and
MySQL reject the negative offset in all three cases, so they would fail there
too. I checked this by compiling the query for each dialect, not by running
those backends.
- With the fix, all three pass. The full `test_xcoms.py` module passes on a
fresh SQLite database: 156 passed.
- `prek run --stage pre-commit` passes on the changed files.
`generate-openapi-spec` and `check-tests-in-the-right-folders` were skipped
because they need Breeze images.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
---
* Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information. Note: commit author/co-author name and email in commits
become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License
Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment:
`{pr_number}.significant.rst`, in
[airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
You can add this file in a follow-up commit after the PR is created so you
know the PR number.
--
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]
