fix(python): align PullRequestLabels and CommitParent composite primary keys with Go domain layer#8957
fix(python): align PullRequestLabels and CommitParent composite primary keys with Go domain layer#8957bujjibabukatta wants to merge 1 commit into
Conversation
|
Hi @klesh could you please review and approve pull request? |
klesh
left a comment
There was a problem hiding this comment.
This doesn't make sense to me. Why adding those extra columns to primary keys?
|
Hi @klesh Thanks for taking a look, and fair pushback — let me walk through why I made this change, since it wasn't obvious from the diff alone. Domain records get saved via session.merge(domain_model) in Convertor._save(). SQLAlchemy's merge() decides insert vs update based on the primary key. With only pull_request_id (or commit_sha) marked as PK, two different labels on the same PR — or two different parents on the same commit — get treated as the same row, so the second merge silently overwrites the first and only one would ever survive. It also matches Go — both fields are already gorm:"primaryKey" there for PullRequestLabel and CommitParent, and the actual DB tables already have this composite PK (Go owns migrations for domain-layer tables, pydevlake doesn't create or alter them). So this isn't a schema change at all — it's just correcting SQLAlchemy's mapper metadata to match what Go already declared and what's already in the database. No migration needed on either side. This file already uses the same composite-PK pattern for PullRequestCommit, RefCommit, and CommitsDiff, so it's consistent with existing conventions too. |
What does this PR do?
Fixes two missing composite primary keys in the Python domain layer that were misaligned with the Go definitions.
Changes
PullRequestLabels.label_name: addedField(primary_key=True)CommitParent.parent_commit_sha: addedField(primary_key=True)Why
Without the correct composite PKs, upserts match on fewer columns than intended,
causing rows that differ only in the missing PK column to silently overwrite
existing records instead of being treated as distinct entries.
Related Issue
Closes #8954