Skip to content

fix: update foreign key constraints and ensure uniqueness in runner metadata#73

Open
johnfolly wants to merge 4 commits intomainfrom
JFO/fix/runner-metadata-fk-link
Open

fix: update foreign key constraints and ensure uniqueness in runner metadata#73
johnfolly wants to merge 4 commits intomainfrom
JFO/fix/runner-metadata-fk-link

Conversation

@johnfolly
Copy link
Copy Markdown

@johnfolly johnfolly commented Apr 8, 2026

This pull request fixes several issues identified in COAL, specifically in the part responsible for sending data to PostgreSQL.

These issues are mainly related to the function that creates the link between the comsotech_runmetadata table and the result tables exported to PostgreSQL.

  • First issue: there was a syntax error in the statement used to define the foreign key constraint. The ADD keyword was missing in the ALTER TABLE statement when declaring the constraint.

  • Second issue: the foreign key constraint was being added to the wrong table name, without the expected prefix. For example, it targeted mytable instead of cosmotech_mytable by default.

  • Third issue: the last_csm_run_id column was missing a uniqueness constraint, which caused the creation of the foreign key constraint to fail.

  • Fourth issue: the code did not check whether the foreign key constraint already existed before attempting to create it. As a result, each write operation tried to recreate the constraint, which is not allowed by PostgreSQL.

  • Fifth and final issue: when the same scenario was executed two or more times, data from the previous run was not deleted. This caused the insertion query into the runner_metadata table to fail because of the uniqueness constraint.

Each of these issues has been fixed and tested in a development environment.

However, some cases are still not handled, such as deleting a scenario, which currently does not remove the related data from the database.

One important remaining point is that we need to find an efficient way to test all these behaviors through integration tests directly in COAL.

@johnfolly johnfolly requested review from lalepee and neomatamune April 9, 2026 08:25
@neomatamune
Copy link
Copy Markdown
Member

Why do we put an overly complicated trigger when we should just do a drop cascade with the foreign keys on upsert (conflict should drop old row and insert new one)

@johnfolly johnfolly force-pushed the JFO/fix/runner-metadata-fk-link branch from 13f0cbd to 57591ea Compare April 9, 2026 09:49
@johnfolly
Copy link
Copy Markdown
Author

Why do we put an overly complicated trigger when we should just do a drop cascade with the foreign keys on upsert (conflict should drop old row and insert new one)

I’ve simplified it

johnfolly and others added 2 commits April 9, 2026 12:32
check on conflict is not necessary anymore as we remove the row beforehand
Copy link
Copy Markdown
Collaborator

@lalepee lalepee left a comment

Choose a reason for hiding this comment

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

Ok for me. I didn't found a better way to do the check on the existence of the foreign key constraint.

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.

3 participants