Skip to content

fix(python): align PullRequestLabels and CommitParent composite primary keys with Go domain layer#8957

Open
bujjibabukatta wants to merge 1 commit into
apache:mainfrom
bujjibabukatta:fix/#8954
Open

fix(python): align PullRequestLabels and CommitParent composite primary keys with Go domain layer#8957
bujjibabukatta wants to merge 1 commit into
apache:mainfrom
bujjibabukatta:fix/#8954

Conversation

@bujjibabukatta

Copy link
Copy Markdown
Contributor

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: added Field(primary_key=True)
  • CommitParent.parent_commit_sha: added Field(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

@bujjibabukatta

Copy link
Copy Markdown
Contributor Author

Hi @klesh could you please review and approve pull request?

@klesh klesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't make sense to me. Why adding those extra columns to primary keys?

@bujjibabukatta

Copy link
Copy Markdown
Contributor Author

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][Pydevlake] Misalignement of Models between Go and Python - missing Primary Keys in PullRequestLabels

2 participants