Fix dj42 issues 66 #78
Conversation
unlike Django using col1, col2 for un-aliased columns we use the same as the field name, this fixes a regression in Django 4.2 which causes the cte columns to to match the table names
|
Our team is waiting on this change to update to Django 4.2. Posting to watch. What is required to get this merged? |
|
As noted in the PR description, this is an invasive set of changes. I have not had time to review it in depth, but I can say that I plan to have a version of django-cte that supports Django 4.2 before 3.2 EOL (April 2024). |
How is the outcome of this PR different from the solution mentioned in #66 (comment)? That is, if CTE fields are aliased using the same name as the field name, how is that different from overriding and passing If they are functionally the same, then I think I would prefer to override and pass One thing I am unclear on is whether |
| if query.combinator: | ||
| return as_sql() | ||
|
|
||
| ctes = [] | ||
| params = [] | ||
| if django.VERSION > (4, 2): |
There was a problem hiding this comment.
>= would be less misleading. While the actual VERSION tuple for 4.2 looks like (4, 2, 0, 'final', 0), a human reader might assume this is intended to exclude 4.2.
(Same on line 162.)
@millerdev With 3.2 now EOL. Just curious to know if you have an idea of the timeline to support 4.2? |
|
@coltonpark5 Except for a few edge cases, #66 is fixed by #81, which was released in v1.3.2. I also reported a bug in Django, but it seems the dev team is not able to maintain compatibility with projects like django-cte that depend on ORM internals. A better solution may be possible if proper I'm going to close this PR since #81 was the preferred fix. |
This add support for Django 4.2 to pass the current test.
In Django 4.2 the SQL of a CTE is build but the columns are automatically asigned an alias this cases the CTE references to fail. This commit attempts to choose beter column aliases so they match the CTE names.
ie Unlike Django using col1, col2 for un-aliased columns we use the same as the field name.
Whist this passes the test it is a invasive set of changes, i did think i'd manages a ultra simple set of changes
The test added is to ensure we don't break the column renaming fix in django 4.2. As the simple removal with_col_aliases in #66 (comment) does to make django_cte pass it test
Fixes: #66