Skip to content
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

fix: Use tuples for pkey check #446

Merged

Conversation

SpaceCondor
Copy link
Contributor

for record in records:
insert_record = {
column.name: record.get(column.name) for column in columns
}
# No need to check for a KeyError here because the SDK already
# guarantees that all key properties exist in the record.
primary_key_value = "".join([str(record[key]) for key in primary_keys])
insert_records[primary_key_value] = insert_record
data_to_insert = list(insert_records.values())

Currently the code above uses string concatenation to check for duplicate primary key values, however this is problematic since the records below will be treated as the same record and the first will be omitted:

Record 1:

  • Primary Key 1: AB
  • Primary Key 2: C

Record 2:

  • Primary Key 1: A
  • Primary Key 2: BC

Changing to a tuple should mitigate this.

@edgarrmondragon edgarrmondragon self-requested a review September 27, 2024 18:42
@edgarrmondragon
Copy link
Member

edgarrmondragon commented Sep 27, 2024

hmm tests are no longer running on PRs from forks...

Nevermind: #447

@edgarrmondragon
Copy link
Member

Thanks @SpaceCondor!

@edgarrmondragon edgarrmondragon merged commit 1a60b45 into MeltanoLabs:main Sep 27, 2024
8 checks passed
@SpaceCondor SpaceCondor deleted the use-tuples-for-pkey-check branch October 1, 2024 15:43
@SpaceCondor SpaceCondor restored the use-tuples-for-pkey-check branch October 1, 2024 15:43
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.

2 participants