-
-
Notifications
You must be signed in to change notification settings - Fork 64
feat(repository): add composite primary key support #640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
cofin
wants to merge
5
commits into
litestar-org:main
Choose a base branch
from
cofin:feat/composite-primary-keys
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds support for composite (multi-column) primary keys in the repository layer. Changes: - Add PrimaryKeyType type alias supporting scalar, tuple, and dict formats - Add helper methods for composite key handling: - _is_composite_pk(): Check if model has composite PK - _build_pk_filter(): Build WHERE clause for PK lookup - _extract_pk_value(): Extract PK value(s) from instance - _pk_values_present(): Check if all PK values are set - _normalize_pk_values_to_tuples(): Convert PK values to tuples for bulk ops - Update get() to support composite keys (tuple or dict input) - Update delete() to support composite keys - Update delete_many() to use tuple_().in_() for efficient bulk operations - Cache PK columns in __init__ for performance The implementation follows SQLAlchemy's native patterns and maintains full backward compatibility for single-column primary keys. Closes litestar-org#189
Extend composite PK support from repository to service layer and memory repositories to complete Phase 3.1 and 3.2 of the composite primary key feature. Changes: - Update service layer get/delete/delete_many signatures to use PrimaryKeyType - Add composite PK helpers to memory repository (_pk_columns, _is_composite_pk, etc.) - Update memory repository get/delete/delete_many for composite keys - Add fallback in _build_pk_filter for mock objects without mapped PKs - Fix unit tests for new code paths Refs: litestar-org#189
Add explicit casts and type annotations to satisfy mypy and pyright strict mode for composite primary key handling methods. Changes: - Add ColumnElement[bool] casts for single-value PK comparisons - Use explicit type annotations instead of redundant casts - Extract type(pk_value).__name__ to local variable for type safety - Add guards for empty tuple edge cases in memory repository
- Replace cast() with explicit type annotations to avoid redundant-cast - Remove unnecessary `not isinstance(pk_value, str)` checks that mypy correctly identifies as unreachable (tuple/dict can't be str subclasses)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #640 +/- ##
==========================================
- Coverage 80.34% 78.58% -1.77%
==========================================
Files 87 87
Lines 6431 6621 +190
Branches 838 883 +45
==========================================
+ Hits 5167 5203 +36
- Misses 1000 1144 +144
- Partials 264 274 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Split combined isinstance check into separate checks to avoid type(pk_value).__name__ which triggers reportUnknownArgumentType - Add type: ignore[redundant-cast] for casts needed by pyright but flagged as redundant by mypy - Add reportUnknownArgumentType and reportUnknownVariableType = false to pyright config for consistency with existing disabled rules
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for composite (multi-column) primary keys across the repository and service layers. This enables working with association tables, legacy databases with natural keys, and any model using multi-column primary keys.
Key Changes
Repository Layer:
PrimaryKeyTypetype alias supporting scalar, tuple, and dict formats_build_pk_filter(),_extract_pk_value(), etc.)get(),delete(), anddelete_many()to support composite keystuple_().in_()for efficient bulk operations with composite keys__init__for performanceService Layer:
get(),delete(), anddelete_many()method signatures to acceptPrimaryKeyTypeMemory Repository:
_pk_columns,_pk_attr_names_is_composite_pk(),_extract_pk_value(),_normalize_pk_to_tuple(),_get_store_key()Usage Examples
Consensus
Implementation approach was validated by gemini-3-pro-preview (9/10 confidence) and gpt-5.2 (8/10 confidence) with the following agreed refinements:
__init__rather than computing per-calltuple_().in_()for bulk delete instead of OR chainsTest Plan
Follow-up Work
update()andupsert()methods for composite key supportRelated Issues
Closes #189