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

DM-43115: Add missing primary keys to DP0.2 schema #253

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

JeremyMcCormick
Copy link
Collaborator

@JeremyMcCormick JeremyMcCormick commented Sep 16, 2024

This fixes errors with foreign key constraints when creating the database for the DP0.2 schema.

The --ignore-constraints flag was removed from the database creation script used in the Github workflows.

@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-43115 branch 4 times, most recently from 8767d21 to ac72dc9 Compare September 16, 2024 19:03
Copy link
Collaborator

@gpdf gpdf left a comment

Choose a reason for hiding this comment

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

OK with the addition of a comment to the YAML for the primary key for the TruthSummary table.

@@ -7872,6 +7880,7 @@ tables:
- name: TruthSummary
'@id': '#TruthSummary'
description: Summary properties of objects from the DESC DC2 truth catalog, as described in arXiv:2101.04855. Includes the noiseless astrometric and photometric parameters.
primaryKey: "#TruthSummary.id_truth_type"
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's OK to do this now, as in practice this attribute seems to be the one used for JOINs and functions "as if" it were the primary unique key, but...

It's not clear why id is not the primary key. The description says it's unique, but in the MatchesTruth table there's a hint that it might not be unique (and so that the truth_type has to be included to achieve uniqueness).

Please add a comment to the YAML to note that anyone re-using this table's schema should think again about whether this is the right choice.

Some foreign key constraints were missing primary key designations
on the target column, causing errors during database instantiation.
This should work for all databases now.
@JeremyMcCormick JeremyMcCormick merged commit a75ee99 into main Sep 27, 2024
9 checks passed
@JeremyMcCormick JeremyMcCormick deleted the tickets/DM-43115 branch September 27, 2024 17:12
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