#33319: Query.change_aliases raises an AssertionError
-------------------------------------+-------------------------------------
Reporter: Ömer Faruk Abacı | Owner: Vishal
| Pandey
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ömer Faruk Abacı):
Replying to [comment:8 Simon Charette]:
> Ömer,
>
> > My initial attempt was to use Query.bump_prefix but as you told it
threw an error.
>
> The error you encountered is due to `bump_prefix` requiring some
adjustments. The alias merging algorithm that `Query.combine` uses
requires that both query share the same initial alias in order to work
otherwise you'll get a `KeyError`. You'll want to find a way to have
`bump_prefix` adjust all the aliases but the initial one so it can still
be used as the merge starting point.
>
> ---
>
> Vishal,
>
> > Here, I am randomly choosing an uppercase letter as the first
character of alias, this returns different aliases, but a test_case is
failing with this approach, with the following traceback.
>
> Choosing a random initial alias will bring more bad than good as not
relying on `alias_prefix` will cause undeterministic failures across the
board. There are reasons why the prefix is used and bumped only on
conflic; it makes reasoning about possible collisions easier.
>
> > In 2nd approach, I have rotated the alias_prefix in a circular manner
from T(class variable alias_prefix has value T) to Z, then from A to Z,
and so on, and it also works on the above-attested code but fails five
other test cases.
>
> This approach doesn't account for possible collisions with subquery
aliases and will eventually collide once you've wrapped up around the 26
letter of the alphabet instead of using the Cartesian product approach.
>
> ---
>
> All of the required logic for preventing `alias_prefix` collisions
already lives in `bump_prefix`, it's only a matter of adjusting it in
order to allow it to be used in the alias merging algorithm of `combine`.
Hello Simon,
I guess I have managed to fix this issue providing a parameter to
`bump_prefix` to prevent changing aliases directly but only bumping the
prefix accordingly. It seems to be working fine, passing all the tests
including the regression tests I have added for this issue. I would
appreciate your reviews for the
[https://github.com/django/django/pull/15128 PR].
Thanks for your effort too Vishal!
--
Ticket URL: <https://code.djangoproject.com/ticket/33319#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/072.d66e9241f52078f733c6a2cfb087b2d7%40djangoproject.com.
