Fix CTE column key resolution for unlabeled expressions in RETURNING clause #12636

IamGroooooot · 2025-05-30T18:12:56Z

Fix CTE column key resolution for unlabeled expressions in RETURNING clause

Description

This PR fixes an issue #12271 where unlabeled expressions in RETURNING clauses lose their column keys when used in CTEs.

Fixes: #12271

Problem

When creating a CTE from DELETE statements with RETURNING clauses containing unlabeled expressions the _make_proxy method got AssertionError because key was being None.

# This fails - because in unlabeled expression key was being None
stmt = delete(t).returning(
    t.c.id,
    t.c.id * -1  # Unlabeled expression
).cte()
assert stmt.c.id.name == "id"  # AssertionError: key was None

Solution

Modified _make_proxy to ensure key is never None by falling back to self._anon_key_label

To validate this fix, the following two test cases have been added:

test_returning_cte_labeled_expression - Ensures labeled expressions continue to work as expected
test_returning_cte_multiple_unlabeled_expressions - Verifies that multiple unlabeled expressions are handled correctly

Checklist

This pull request is:

A documentation / typographical / small typing error fix
- Good to go, no issue or tests are needed
A short code fix
- please include the issue number, and create an issue if none exists, which
  must include a complete example of the issue. one line code fixes without an
  issue and demonstration will not be accepted.
- Please include: Fixes: #<issue number> in the commit message
- please include tests. one line code fixes without tests will not be accepted.
A new feature implementation
- please include the issue number, and create an issue if none exists, which must
  include a complete example of how the feature would look.
- Please include: Fixes: #<issue number> in the commit message
- please include tests.

Have a nice day!

Fixes: sqlalchemy#12271

CaselIT

Thanks for opening this

CaselIT · 2025-05-30T20:26:08Z

test/sql/test_returning.py

+        ).cte()
+
+        assert stmt.c.id is not None
+        assert all(col is not None for col in stmt.c)


we could also assert that the len of stmt.c is the expected one (5)

Nice 👍. added the len(stmt.c) check!

IamGroooooot added 2 commits May 31, 2025 03:10

test: add test for CTE with unlabeled and labeled expressions

e6236b3

fix: CTE key resolution for unlabeled expressions in RETURNING clause

9f4963b

Fixes: sqlalchemy#12271

CaselIT reviewed May 30, 2025

View reviewed changes

CaselIT requested a review from zzzeek May 30, 2025 20:27

IamGroooooot added 3 commits May 31, 2025 10:49

style: fix E501 line too long

58fdcb8

test: add assertion for the number of columns in RETURNING CTE statement

80fff29

style: reformat test_returning to pass black check

6f80a59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix CTE column key resolution for unlabeled expressions in RETURNING clause #12636

Fix CTE column key resolution for unlabeled expressions in RETURNING clause #12636

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fix CTE column key resolution for unlabeled expressions in RETURNING clause #12636

Are you sure you want to change the base?

Fix CTE column key resolution for unlabeled expressions in RETURNING clause #12636

Uh oh!

Conversation

Uh oh!

Description

Problem

Solution

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!