[Django] #27833: prefetch_related fails with SQLite when used with 1000 parent records
Django
-------------------------------------+-------------------------------------
Reporter: Jason | Owner: nobody
Barnabe |
Type: Bug | Status: new
Component: Database | Version: 1.10
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
This is described in
https://code.djangoproject.com/ticket/16937#comment:3, but I don't see any
issue filed for it.
"[prefetch_related] will not work on some backends if you have a lot of
objects in your queryset. For example the SQLite backend has a limitation
of 999 parameters to single SQL query, so if you have 1000 objects,
prefetch_related will fail as you need to supply 1000 id values to the
query."'
Batch it up like in #16426 and #17788, which dealt with the same
limitation?
--
Ticket URL: <https://code.djangoproject.com/ticket/27833>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 1.10
(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
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Old description:
> This is described in
> https://code.djangoproject.com/ticket/16937#comment:3, but I don't see
> any issue filed for it.
>
> "[prefetch_related] will not work on some backends if you have a lot of
> objects in your queryset. For example the SQLite backend has a limitation
> of 999 parameters to single SQL query, so if you have 1000 objects,
> prefetch_related will fail as you need to supply 1000 id values to the
> query."'
>
> Batch it up like in #16426 and #17788, which dealt with the same
> limitation?
New description:
This is described in ticket:16937#comment:3, but I don't see any issue
filed for it.
"[prefetch_related] will not work on some backends if you have a lot of
objects in your queryset. For example the SQLite backend has a limitation
of 999 parameters to single SQL query, so if you have 1000 objects,
prefetch_related will fail as you need to supply 1000 id values to the
query."'
Batch it up like in #16426 and #17788, which dealt with the same
limitation?
--
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:1>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => Raphael Michel
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:2>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:3>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:4>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:5>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:6>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:7>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:8>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Ready for checkin => Accepted
Comment:
As mentioned in the PR, I think there is one major issue:
{{{prefetch_related}}} expects the results of the prefetch query to be
unique.
Splitting the query in batches do not enforce uniqueness and can lead to
incorrect results. In short, if related objects are shared between several
instances, when instances are split into batches, nothing prevents a
related objects from appearing twice in the results. For example:
{{{
# Suppose there are 2 instances (A, B) and 2 related objects (1, 2)
A -> 1
B -> 2
A -> 1
}}}
Prefetching in batches of 2 results in prefetch queries for: {{{[A, B]}}}
then {{{[C]}}}.
Which gives the following mapping of related objects: {{{[(A, 1), (B,
2)]}}} then {{{[(A, 1)]}}}. The {{{rel_obj_cache}}} would then be:
{{{#!python
{
(1,): [A, A], # A should only be present once
(2,): [B]
}
}}}
A more detailed example is available on the PR.
Maybe the way forward is to use a set or a dict to store the related
queries' results (either for {{{all_related_objects}}} or for
{{{rel_obj_cache}}}).
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:9>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:10>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Raphael Michel => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:11>
Django
-------------------------------------+-------------------------------------
| Donthi
Type: Bug | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Yashas Donthi
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:12>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
* owner: Yashas Donthi => (none)
Two notes worth of mention here.
1. In the case of records referenced through multiple fields (e.g.
`ForeignObject(from_felds, to_fields)` where more than one field is
involved) then the problem can manifest itself with `(1000 / len(fields))
+ 1` records as SQLite doesn't support tuple comparison and thus must
expand `(field0, ..., fieldN) IN %s` into `(field0 = v0_0 ..., fieldN =
vn_0) ... OR ... (field0 = v0_m ..., fieldN = vn_m)`. See #36116 where
this was uncovered.
2. Now that we changed the prefetching interface from
`get_prefetch_queryset` to `get_prefetch_querysets` in #33651 it paves the
way for prefetcher implementations to be adapted to slice `instances`
based on `connection.ops.bulk_batch_size` which makes solving this ticket
way easier.
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:13>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Another important point is that [https://sqlite.org/releaselog/3_32_0.html
SQLite 3.32] (released on 2020-05-22) increased the
[https://sqlite.org/limits.html#max_variable_number default upper bound]
on the number of [https://sqlite.org/lang_expr.html#varparam parameters]
from 999 to 32766 so while there is still a limit that warrants the usage
of `bulk_batch_size` users are way less likely to run into this issue than
before.
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:14>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
#36707 was a dupe.
--
Ticket URL: <https://code.djangoproject.com/ticket/27833#comment:15>