Repository navigation
fix(heading): keep table_info in set_primary_key so a later rename quotes for the backend - #1643
mayuriphad wants to merge 1 commit into
Conversation
Sorry, something went wrong.
…otes for the backend Signed-off-by: mayuriphad <mayuriphad656@gmail.com>
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Sorry, something went wrong.
|
Thanks, @mayuriphad. This is the fix for #1638, and the root cause matches what I found when filing it: set_primary_key dropped table_info. Fork PRs don't trigger CI in this repository at the moment, so I ran it locally on current master:
66 passed in all. A maintainer will follow up on the review. |
Sorry, something went wrong.
This branch has not been deployed
Reviewers
At least 1 approving review is required to merge this pull request.
Fixes #1638
Heading.set_primary_key built the new heading without table_info, so it lost the connection. When a projection then renamed an attribute of that heading, Heading.select had no adapter and fell back to backtick quoting. That is why (dj.U("b") & T).proj(c="b") compiled to SELECT DISTINCT `b` as "c" and failed on PostgreSQL. MySQL accepts backticks, so it hid the problem there.
set_primary_key now passes table_info=self.table_info, the same way select already hands it on to the heading it builds.
The new unit test in tests/unit/test_pk_rules.py gives a heading a PostgreSQL adapter, calls set_primary_key, then renames an attribute. On master the expression comes out as `b`. With the fix it is "b". tests/unit passes locally (458 passed). I did not run the integration suite because there is no Docker on this machine for the testcontainers.