#36934: BuiltinLookup breaks with params-as-a-tuple in django 6.0
-------------------------------------+-------------------------------------
Reporter: Stefan Bühler | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Stefan Bühler):
* resolution: needsinfo =>
* status: closed => new
Comment:
Replying to [comment:2 Jacob Walls]:
> Hi, thanks for the report, and thanks Clifford for triage. Clifford, I
don't know if you saw the closely related #36922, which we closed since it
was the responsibility of the custom lookup to return params in a tuple.
That is what is written in the release notes, but the reality is
overwrites of `BuiltinLookup.process_lhs` need to return a list (NOT a
tuple) or it breaks `BuiltinLookup.as_sql`.
> We chose not to backport since this should only be an issue if you've
subclassed the lookups and added further customizations. Your link doesn't
go to a custom lookup, so I can't tell how you're affected. Could you
please include a stacktrace and the custom lookup?
What do you mean "Your link doesn't go to a custom lookup"? The linked
django-netfields lines are obviously custom lookups, which are registered
with `CidrAddressField` and `InetAddressField` (I hope you don't need the
link for that), and they are used with something like this:
`CharFilter(field_name='cidr_address_field', lookup_expr='iregex')`.
> We have to weigh the potential upside from backporting against the
downside of introducing unexpected problems in projects that already
tested against the 6.0 betas. Eager to see the fuller stacktrace & lookup,
and we can look again at that point. Thanks.
The backtrace is not interesting at all, since it just points to
`params.extend(rhs_params)` in `BuiltinLookup.as_sql` (about 10 frames
deep in django); I traced it down to where the tuple params came from
(i.e. `netfields.lookups.IRegex`), and that is what I reported here. Also
it is just obvious from the code it can't work that way.
Now the linked pull-request not only replaces the broken `params.extend`
lines but also changes the return type from list to tuple in a few places
- I suppose the latter ones could be omitted in the backport, but
`BuiltinLookup.as_sql` (and `YearLookup.as_sql`) really needs to handle
tuple params from `process_lhs`.
If you still think this isn't your bug to fix I'll forward this to
netfields; I already replaced the lookup in my own code so I don't really
need a fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/36934#comment:3>
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 visit
https://groups.google.com/d/msgid/django-updates/0107019c7636a019-1e8d692d-c0b5-4139-96f5-5568a35586d3-000000%40eu-central-1.amazonses.com.
