Recursive and other "combinator" queries broken in django-cte

[Daniel Miller]

70499b2 in #34123 ​broke django-cte. The problem is caused by compiler.as_sql(with_col_aliases=True) in get_combinator_sql: with_col_aliases=True breaks the ability to reference CTE query columns by name on Django 4.2.

[Natalia Bidart]

Hello Daniel, thanks for your report.

I read the referenced issues and skimmed the related PRs. As far as I know, there is no much that can be done in the Django side, this ticket tracker is to track bugs and issues with Django itself and from the provided description and related links, it's unclear how/if Django has a bug in its code. I understand that the referenced revision adding the column aliases broke django-cte, but it legitimately fixed confirmed issues *within* Django, so following the current ​ticket triaging guidelines, I think we need to close this report as invalid.

I'll cc Mariusz and Simon who were involved in fixing ticket #34123 in case they disagree with this resolution, or in case they could provide insights about possible fixes for django-cte (though the latter would be more appropriate to be ​a new post in the Django Internals forum).

[Daniel Miller]

Hi Natalia, thanks for your prompt response.

I could have done a better job explaining why I think this is a bug in Django. As of 70499b2 all "combinator" queries (queries using UNION, etc.) now have compiler-generated aliases col1, col2, ... applied to unaliased¹ columns. This change was made to support a small subset of queries that use a combinator with ORDER BY and select_related. However, the fix as implemented also unnecessarily affects all other combinator queries as well. Frankly I am surprised that it did not break anything else outside of django-cte. Is there a way to implement a fix so that it only affects queries where aliases are needed?

In my attempts to fix the issue in django-cte I have been unable to find a way to cleanly disable compiler-level aliasing just for CTE queries where it breaks the ability to reference columns by name. One reason is that many ORM methods like get_combinator_sql are large and overriding them is difficult or impossible to do in a maintainable way. I am open to suggestions for how to accomplish this within django-cte if there is a good way to do that.

I think it is in Django's interest to make it possible for projects like django-cte to extend the functionality of the ORM with new features, since the alternative is to have all such features implemented internally and maintained as part of Django or have them be repeatedly broken as Django evolves over time.

¹ The definition of "unaliased" here is a bit murky. There are at least two ways a column can be aliased: explicitly at the Query level, or implicitly at the Compiler level when with_col_aliases=True. Compiler-level aliases do not mask or override Query-level aliases. My understanding of "unaliased" is all columns without a Query-level alias.

[Natalia Bidart]

Thank you Daniel for the extra rationale, that definitely helps. Mariusz is currently returning from DjangoCon Africa, but I'm sure he'll share his opinion next week when catching up with Django things. Simon also reads issues periodically, so I'm very keen to wait and read their thoughts since they are the experts of this Django area.

[Simon Charette]

Hello Daniel!

I sympathize with the desire to avoid duplicating large chunks of code in django-cte and I appreciate that you maintain such a solution but asking for the development team to maintain compatibility with its internal interface when adjusting bugs of the interface it maintains is not sustainable particularly for regressions in versions that have been released for a few months already. When making these changes we try to think of ways that are not too invasive but there's only so much we can do to account for all cases.

I'm very much open to reviewing patches that would reduce the maintenance burden that you face (is there a kwarg that can be passed? a different function breakdown be used?) in main or that implements smarter way of doing things that avoids unnecessary aliasing such as proper ORDER BY $index support but stability of the ORM internals cannot be guaranteed by team in its current state.

Is there a way to implement a fix so that it only affects queries where aliases are needed?

I believe that if you build and maintain a package that relies on the stability of ORM internals you must be prepared to deal with regressions against main and suggest alternative solutions when they happen. I'm afraid the ship has already sailed for the 4.2 backport support window and I'm not even sure it would qualify given the internal nature of SQL compilation.