Skip to content

feat: add catalog search, ordering, and trigram indexes - #451

Open
TenSt wants to merge 1 commit into
pulp:mainfrom
TenSt:stepan/450-catalog-search-ordering-and-indexes
Open

feat: add catalog search, ordering, and trigram indexes#451
TenSt wants to merge 1 commit into
pulp:mainfrom
TenSt:stepan/450-catalog-search-ordering-and-indexes

Conversation

@TenSt

@TenSt TenSt commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📜 Checklist

  • Commits are cleanly separated with meaningful messages (simple features and bug fixes should be squashed to one commit)
  • A changelog entry or entries has been added for any significant changes
  • [x Follows the Pulp policy on AI Usage
  • (For new features) - User documentation and test coverage has been added

Summary

This PR:

  • Adds search on repository packages/ (contains on groupId/artifactId; group:artifact ANDs both)
  • Adds last_updated and ordering so clients can sort the catalog
  • Lists versions newest-first using numeric version order
  • Adds pg_trgm GIN indexes on group_id and artifact_id for ILIKE filters
  • Treats a trailing .letters-... last-segment suffix as a rebuild qualifier, so 5.3.17.rhlw-00001 and 5.3.17.rhlw-00001-n0001 share base version 5.3.17

Closes #450
Closes #459

@TenSt
TenSt force-pushed the stepan/450-catalog-search-ordering-and-indexes branch 2 times, most recently from 7ac6427 to b92e8f9 Compare September 7, 2026 20:53
@TenSt
TenSt force-pushed the stepan/450-catalog-search-ordering-and-indexes branch 2 times, most recently from 4111cfc to 585c381 Compare September 11, 2026 17:09
Add a search parameter on the repository package catalog so clients can match
groupId and artifactId by contains, including group:artifact. Support last_updated
ordering and newest-first versions, and add pg_trgm GIN indexes for ILIKE filters.

Assisted-By: Cursor
@TenSt
TenSt force-pushed the stepan/450-catalog-search-ordering-and-indexes branch from 585c381 to d67bd2b Compare September 11, 2026 17:20

@dkliban dkliban left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

1. Migration will block writes (needs fix)

migrations.AddIndex takes an ACCESS EXCLUSIVE lock — blocking all reads and writes on core_mavenpackage while the index builds. Use AddIndexConcurrently instead (the same pattern used in pulpcore's 0147_content_pulp_labels_gin.py):

from django.contrib.postgres.operations import AddIndexConcurrently, TrigramExtension

class Migration(migrations.Migration):
    atomic = False  # required for CONCURRENTLY

    operations = [
        TrigramExtension(),
        AddIndexConcurrently(
            model_name="mavenpackage",
            index=GinIndex(fields=["group_id"], name="maven_pkg_group_id_trgm", opclasses=["gin_trgm_ops"]),
        ),
        AddIndexConcurrently(
            model_name="mavenpackage",
            index=GinIndex(fields=["artifact_id"], name="maven_pkg_artifact_id_trgm", opclasses=["gin_trgm_ops"]),
        ),
    ]

Note: atomic = False means no automatic rollback — a failed AddIndexConcurrently may leave an invalid index requiring manual DROP INDEX CONCURRENTLY IF EXISTS before re-running.

pg_trgm is a trusted extension on RDS PostgreSQL 13+ so TrigramExtension() will work fine with the application DB user — no DBA intervention needed.

2. Lint failure in CI

ruff reports I001 (unsorted imports) in pulp_maven/app/migrations/0010_mavenpackage.py:3pulpcore.app.util must move after the django imports. Fix with:

ruff check --fix pulp_maven/app/migrations/0010_mavenpackage.py

@dkliban dkliban left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction to my earlier review: I was wrong about TrigramExtension() requiring superuser on RDS. According to AWS docs, pg_trgm is a trusted extension on PostgreSQL 13+ — any user with CREATE privilege on the database can install it, no rds_superuser needed. The TrigramExtension() call in the migration is fine as-is.

The two remaining concerns stand: blocking index creation (use AddIndexConcurrently) and the lint failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants