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: prevent duplicate entries with overlapping block_ranges #316

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

Sekhmet
Copy link
Contributor

@Sekhmet Sekhmet commented Oct 9, 2024

It was possible to create duplicate entires that would apply within the same block that would have single ID.

That shouldn't be the case, ID is not longer unique in whole table, but should be within single block_range.

It was possible to create duplicate entires that would apply
within the same block that would have single ID.

That shouldn't be the case, ID is not longer unique in whole table,
but should be within single block_range.
@Sekhmet Sekhmet requested a review from bonustrack October 9, 2024 11:32
@@ -205,6 +205,10 @@ export class GqlEntityController {
public async createEntityStores(knex: Knex): Promise<{ builder: Knex.SchemaBuilder }> {
let builder = knex.schema;

if (knex.client.config.client === 'pg') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we support other db than Postgres? Or what is this condition for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment just for our tests as those run with SQLite.

Copy link
Contributor

@bonustrack bonustrack left a comment

Choose a reason for hiding this comment

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

utACK

@Sekhmet Sekhmet merged commit 6221ea4 into master Oct 9, 2024
1 check passed
@Sekhmet Sekhmet deleted the sekhmet/duplicate-detections branch October 9, 2024 12:57
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