From 655ace0c8be727194dd6a3ecfd376ca6eff1cb79 Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Tue, 16 Jun 2026 12:42:55 -0400 Subject: [PATCH 1/3] Add safe_in() utility to prevent PostgreSQL 65K parameter limit errors PostgreSQL's wire protocol limits bind parameters to 65,535 per statement. When Django ORM's filter(field__in=python_list) generates WHERE field IN ($1, $2, ..., $65536+), it exceeds this limit when using server-side cursors (.iterator()). This introduces a safe_in() utility that uses a custom Django lookup (= ANY(%s)) for large lists, passing the entire list as a single PostgreSQL array parameter regardless of size. For small lists, the standard __in lookup is used unchanged. Applied safe_in() to all vulnerable code paths in pulpcore: - RepositoryVersion.get_content(), added(), removed() - import_repository_version() content mapping Also updated the test to use .iterator() so it reliably exercises the server-side cursor path that triggers the parameter limit. Assisted-By: claude-opus-4.6 --- CHANGES/+safe-in-parameter-limit.bugfix | 1 + .../+safe-in-parameter-limit.feature | 1 + pulpcore/app/models/repository.py | 35 ++++++--------- pulpcore/app/tasks/importer.py | 5 ++- pulpcore/app/util.py | 43 ++++++++++++++++++- pulpcore/plugin/util.py | 2 + 6 files changed, 63 insertions(+), 24 deletions(-) create mode 100644 CHANGES/+safe-in-parameter-limit.bugfix create mode 100644 CHANGES/plugin_api/+safe-in-parameter-limit.feature diff --git a/CHANGES/+safe-in-parameter-limit.bugfix b/CHANGES/+safe-in-parameter-limit.bugfix new file mode 100644 index 00000000000..f6ff013e232 --- /dev/null +++ b/CHANGES/+safe-in-parameter-limit.bugfix @@ -0,0 +1 @@ +Avoid exceeding PostgreSQL's 65,535 query parameter limit when filtering by large lists of IDs. This fixes `OperationalError` crashes during large import and copy operations involving more than 65,535 content units. \ No newline at end of file diff --git a/CHANGES/plugin_api/+safe-in-parameter-limit.feature b/CHANGES/plugin_api/+safe-in-parameter-limit.feature new file mode 100644 index 00000000000..8ef75db667a --- /dev/null +++ b/CHANGES/plugin_api/+safe-in-parameter-limit.feature @@ -0,0 +1 @@ +Added `safe_in()` to the plugin API for building `Q` objects that are safe for arbitrarily large value lists, avoiding PostgreSQL's 65,535 query parameter limit. \ No newline at end of file diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index 5be034f5ffa..d64a3e669c7 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -25,6 +25,7 @@ get_prn, get_view_name_for_model, reverse, + safe_in, ) from pulpcore.cache import Cache from pulpcore.constants import ALL_KNOWN_CONTENT_CHECKSUMS, PROTECTED_REPO_VERSION_MESSAGE @@ -981,7 +982,7 @@ def get_content(self, content_qs=None): Args: content_qs (django.db.models.QuerySet): The queryset for Content that will be restricted further to the content present in this repository version. If not given, - ``Content.objects.all()`` is used (to return over all content types present in the + `Content.objects.all()` is used (to return over all content types present in the repository version). Returns: @@ -997,15 +998,7 @@ def get_content(self, content_qs=None): if content_qs is None: content_qs = Content.objects - content_ids = self.content_ids - if len(content_ids) >= 65535: - # Workaround for PostgreSQL's limit on the number of parameters in a query - content_ids = ( - RepositoryVersion.objects.filter(pk=self.pk) - .annotate(cids=Func(F("content_ids"), function="unnest")) - .values_list("cids", flat=True) - ) - return content_qs.filter(pk__in=content_ids) + return content_qs.filter(safe_in("pk", self.content_ids)) @property def content(self): @@ -1049,14 +1042,14 @@ def content_batch_qs(self, content_qs=None, order_by_params=("pk",), batch_size= Args: content_qs (django.db.models.QuerySet) The queryset for Content that will be restricted further to the content present in this repository version. If not given, - ``Content.objects.all()`` is used (to iterate over all content present in the + `Content.objects.all()` is used (to iterate over all content present in the repository version). A plugin may want to use a specific subclass of - [pulpcore.plugin.models.Content][] or use e.g. ``filter()`` to select + [pulpcore.plugin.models.Content][] or use e.g. `filter()` to select a subset of the repository version's content. - order_by_params (tuple of str): The parameters for the ``order_by`` clause - for the content. The Default is ``("pk",)``. This needs to + order_by_params (tuple of str): The parameters for the `order_by` clause + for the content. The Default is `("pk",)`. This needs to specify a stable order. For example, if you want to iterate by - decreasing creation time stamps use ``("-pulp_created", "pk")`` to + decreasing creation time stamps use `("-pulp_created", "pk")` to ensure that content records are still sorted by primary key even if their creation timestamp happens to be equal. batch_size (int): The maximum batch size. @@ -1065,8 +1058,8 @@ def content_batch_qs(self, content_qs=None, order_by_params=("pk",), batch_size= [django.db.models.QuerySet][]: A QuerySet representing a slice of the content. Example: - The following code could be used to loop over all ``FileContent`` in - ``repository_version``. It prefetches the related + The following code could be used to loop over all `FileContent` in + `repository_version`. It prefetches the related [pulpcore.plugin.models.ContentArtifact][] instances for every batch:: repository_version = ... @@ -1119,8 +1112,8 @@ def added(self, base_version=None): if not base_version: return Content.objects.filter(version_memberships__version_added=self) - return Content.objects.filter(pk__in=self.content_ids).exclude( - pk__in=base_version.content_ids + return Content.objects.filter(safe_in("pk", self.content_ids)).exclude( + safe_in("pk", base_version.content_ids) ) def removed(self, base_version=None): @@ -1134,8 +1127,8 @@ def removed(self, base_version=None): if not base_version: return Content.objects.filter(version_memberships__version_removed=self) - return Content.objects.filter(pk__in=base_version.content_ids).exclude( - pk__in=self.content_ids + return Content.objects.filter(safe_in("pk", base_version.content_ids)).exclude( + safe_in("pk", self.content_ids) ) def contains(self, content): diff --git a/pulpcore/app/tasks/importer.py b/pulpcore/app/tasks/importer.py index 3fa4f00d390..e3adb3fb9fd 100644 --- a/pulpcore/app/tasks/importer.py +++ b/pulpcore/app/tasks/importer.py @@ -39,6 +39,7 @@ compute_file_hash, get_domain, get_domain_pk, + safe_in, ) from pulpcore.constants import TASK_STATES from pulpcore.exceptions.plugin import MissingPlugin @@ -417,14 +418,14 @@ def import_repository_version( for repo_name, content_ids in mapping.items(): repo_name = _get_destination_repo_name(importer, repo_name) dest_repo = Repository.objects.get(name=repo_name) - content = Content.objects.filter(upstream_id__in=content_ids) + content = Content.objects.filter(safe_in("upstream_id", content_ids)) content_count += len(content_ids) with dest_repo.new_version() as new_version: new_version.set_content(content) else: # just map all the content to our destination repo dest_repo = Repository.objects.get(pk=dest_repo_pk) - content = Content.objects.filter(pk__in=resulting_content_ids) + content = Content.objects.filter(safe_in("pk", resulting_content_ids)) content_count += len(resulting_content_ids) with dest_repo.new_version() as new_version: new_version.set_content(content) diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index 76f2a5b47fb..e0564455240 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -16,7 +16,7 @@ from django.apps import apps from django.conf import settings from django.db import connection -from django.db.models import Model, UUIDField +from django.db.models import Field, Lookup, Model, Q, UUIDField from rest_framework.reverse import reverse as drf_reverse from rest_framework.serializers import ValidationError @@ -26,6 +26,47 @@ from pulpcore.app.loggers import deprecation_logger from pulpcore.exceptions.validation import InvalidSignatureError +POSTGRES_MAX_QUERY_PARAMS = 65535 + + +class AnyArray(Lookup): + """PostgreSQL `= ANY(%s)` lookup that passes a list as a single array parameter. + + psycopg3 adapts the Python list into a PostgreSQL array, so the entire list + counts as **one** bind parameter regardless of size. This avoids the + protocol-level 65535-parameter limit that `IN ($1, $2, …)` hits. + """ + + lookup_name = "any_array" + + def get_prep_lookup(self): + return [self.lhs.output_field.get_prep_value(v) for v in self.rhs] + + def as_sql(self, compiler, connection): + lhs, lhs_params = self.process_lhs(compiler, connection) + return f"{lhs} = ANY(%s)", lhs_params + [list(self.rhs)] + + +Field.register_lookup(AnyArray) + + +def safe_in(field_name, values): + """Build a `Q` object for `field__in` that is safe for arbitrarily large lists. + + * If *values* is already a queryset (or other non-collection type), the + normal `__in` lookup is used — Django turns it into a subquery. + * If the collection has fewer than 65 535 items, `__in` is used as-is. + * Otherwise `__any_array` is used so the whole list travels as a single + PostgreSQL array parameter. + """ + if not isinstance(values, (list, set, tuple, frozenset)): + return Q(**{f"{field_name}__in": values}) + values = list(values) + if len(values) < POSTGRES_MAX_QUERY_PARAMS: + return Q(**{f"{field_name}__in": values}) + return Q(**{f"{field_name}__any_array": values}) + + # a little cache so viewset_for_model doesn't have to iterate over every app every time _model_viewset_cache = {} diff --git a/pulpcore/plugin/util.py b/pulpcore/plugin/util.py index b8e63db206f..4a9cd1d5dd9 100644 --- a/pulpcore/plugin/util.py +++ b/pulpcore/plugin/util.py @@ -27,6 +27,7 @@ raise_for_unknown_content_units, resolve_prn, reverse, + safe_in, set_current_user, set_domain, ) @@ -59,5 +60,6 @@ "reverse", "set_current_user", "resolve_prn", + "safe_in", "cache_key", ] From 74a16c2d96a4ccbb37f894013e72a7dd4b6ba22b Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Thu, 30 Jul 2026 23:55:58 -0400 Subject: [PATCH 2/3] temp --- pulpcore/app/models/repository.py | 39 +++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index d64a3e669c7..38b7d45c24f 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -25,7 +25,6 @@ get_prn, get_view_name_for_model, reverse, - safe_in, ) from pulpcore.cache import Cache from pulpcore.constants import ALL_KNOWN_CONTENT_CHECKSUMS, PROTECTED_REPO_VERSION_MESSAGE @@ -909,6 +908,19 @@ def with_content(self, content): return self.filter(content_ids__overlap=content_pks) +class RepositoryVersionManager(models.Manager): + """Manager that defers the content_ids array column by default. + + The content_ids array can be very large and is expensive to transfer from + PostgreSQL to Python. Most queries don't need it — callers that do + (add_content, remove_content, set_content_ids, etc.) access it through + the model instance which triggers a deferred-field load automatically. + """ + + def get_queryset(self): + return RepositoryVersionQuerySet(self.model, using=self._db).defer("content_ids") + + class RepositoryVersion(BaseModel): """ A version of a repository's content set. @@ -937,7 +949,7 @@ class RepositoryVersion(BaseModel): base_version (models.ForeignKey): The repository version this was created from. """ - objects = RepositoryVersionQuerySet.as_manager() + objects = RepositoryVersionManager() repository = models.ForeignKey(Repository, on_delete=models.CASCADE) number = models.PositiveIntegerField(db_index=True) @@ -975,6 +987,19 @@ def set_content_ids(self): else: self.content_ids = previous.content_ids + def _content_ids_subquery(self): + """Return a subquery that unnests content_ids server-side. + + Keeps the array data inside PostgreSQL, avoiding the round-trip of + loading a potentially huge list into Python and sending it back as + query parameters. + """ + return ( + RepositoryVersion.objects.filter(pk=self.pk) + .annotate(cids=Func(F("content_ids"), function="unnest")) + .values_list("cids", flat=True) + ) + def get_content(self, content_qs=None): """ Returns a set of content for a repository version @@ -998,7 +1023,7 @@ def get_content(self, content_qs=None): if content_qs is None: content_qs = Content.objects - return content_qs.filter(safe_in("pk", self.content_ids)) + return content_qs.filter(pk__in=self._content_ids_subquery()) @property def content(self): @@ -1112,8 +1137,8 @@ def added(self, base_version=None): if not base_version: return Content.objects.filter(version_memberships__version_added=self) - return Content.objects.filter(safe_in("pk", self.content_ids)).exclude( - safe_in("pk", base_version.content_ids) + return Content.objects.filter(pk__in=self._content_ids_subquery()).exclude( + pk__in=base_version._content_ids_subquery() ) def removed(self, base_version=None): @@ -1127,8 +1152,8 @@ def removed(self, base_version=None): if not base_version: return Content.objects.filter(version_memberships__version_removed=self) - return Content.objects.filter(safe_in("pk", base_version.content_ids)).exclude( - safe_in("pk", self.content_ids) + return Content.objects.filter(pk__in=base_version._content_ids_subquery()).exclude( + pk__in=self._content_ids_subquery() ) def contains(self, content): From bd3abb17f1db900bf022a6f325af1f7e72e70bbd Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Thu, 30 Jul 2026 23:58:25 -0400 Subject: [PATCH 3/3] temp2 --- pulpcore/app/models/repository.py | 4 ++-- pulpcore/app/util.py | 29 +++++++++++++++-------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index 38b7d45c24f..c34a74de986 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -908,7 +908,7 @@ def with_content(self, content): return self.filter(content_ids__overlap=content_pks) -class RepositoryVersionManager(models.Manager): +class RepositoryVersionManager(models.Manager.from_queryset(RepositoryVersionQuerySet)): """Manager that defers the content_ids array column by default. The content_ids array can be very large and is expensive to transfer from @@ -918,7 +918,7 @@ class RepositoryVersionManager(models.Manager): """ def get_queryset(self): - return RepositoryVersionQuerySet(self.model, using=self._db).defer("content_ids") + return super().get_queryset().defer("content_ids") class RepositoryVersion(BaseModel): diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index e0564455240..50bbd654a33 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -26,15 +26,13 @@ from pulpcore.app.loggers import deprecation_logger from pulpcore.exceptions.validation import InvalidSignatureError -POSTGRES_MAX_QUERY_PARAMS = 65535 - class AnyArray(Lookup): - """PostgreSQL `= ANY(%s)` lookup that passes a list as a single array parameter. + """PostgreSQL ``= ANY(%s)`` lookup that passes a list as a single array parameter. psycopg3 adapts the Python list into a PostgreSQL array, so the entire list counts as **one** bind parameter regardless of size. This avoids the - protocol-level 65535-parameter limit that `IN ($1, $2, …)` hits. + protocol-level 65,535-parameter limit that ``IN ($1, $2, …)`` hits. """ lookup_name = "any_array" @@ -51,20 +49,23 @@ def as_sql(self, compiler, connection): def safe_in(field_name, values): - """Build a `Q` object for `field__in` that is safe for arbitrarily large lists. + """Build a ``Q`` object for filtering by a list of values that is safe at any size. + + Uses PostgreSQL's ``= ANY(array)`` syntax so the entire list is sent as a + single bind parameter, avoiding the 65,535-parameter protocol limit that + Django's default ``__in`` lookup hits with large lists. - * If *values* is already a queryset (or other non-collection type), the - normal `__in` lookup is used — Django turns it into a subquery. - * If the collection has fewer than 65 535 items, `__in` is used as-is. - * Otherwise `__any_array` is used so the whole list travels as a single - PostgreSQL array parameter. + Use this for values that are **already materialised in Python** (e.g. + parsed from JSON, accumulated in a loop). When the IDs live in a database + column, prefer a subquery instead — it keeps the data server-side and + avoids the round-trip entirely. + + If *values* is a queryset (or other non-collection type), falls back to + the normal ``__in`` lookup, which Django turns into a subquery. """ if not isinstance(values, (list, set, tuple, frozenset)): return Q(**{f"{field_name}__in": values}) - values = list(values) - if len(values) < POSTGRES_MAX_QUERY_PARAMS: - return Q(**{f"{field_name}__in": values}) - return Q(**{f"{field_name}__any_array": values}) + return Q(**{f"{field_name}__any_array": list(values)}) # a little cache so viewset_for_model doesn't have to iterate over every app every time