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

user_id constraint added to webpage activity table #3428

Merged
merged 14 commits into from
Dec 6, 2023
Merged

Conversation

dylanbun
Copy link
Collaborator

Resolves #2091

Added a foreign key constraint to the user_id column in webpage activity. Had to adjust temporary_label_id in a couple format files from nullable to non-null.

Testing instructions
  1. Use preferred choice of database management tool
  2. Check webpage_activity table for any rows with a user_id being NULL or a user_id not in the sidewalk_user table. Should be 0
Things to check before submitting the PR
  • I've written a descriptive PR title.

@dylanbun dylanbun self-assigned this Nov 27, 2023
@misaugstad
Copy link
Member

Had to adjust temporary_label_id in a couple format files from nullable to non-null.

I don't think that the temporary_label_id is related, is it? It looks more like a hold over of the code that we decided needed to be removed from your previous PR #3408.

Another small thing is that you actually shouldn't update schema.sql! When we create a new database, we start from that seed database. Then all the "evolutions" are run on it! So the schema should be frozen in time.

@dylanbun
Copy link
Collaborator Author

cleaned it up, ready for review @misaugstad

@misaugstad misaugstad self-assigned this Dec 1, 2023
@@ -0,0 +1,7 @@
# --- !Ups
DELETE FROM webpage_activity USING sidewalk_user WHERE webpage_activity.user_id = sidewalk_user.user_id AND sidewalk_user.user_id IS NULL;
Copy link
Member

Choose a reason for hiding this comment

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

I think this will only delete entries if the user_id is null, bc it's doing an inner join? I threw your original query into chatGPT and asked it to generate one that uses a join, and it looks right to me.

My question:

translate this into using a join with the sidewalk_user table instead of as a subquery
DELETE FROM webpage_activity WHERE user_id IS NULL OR user_id NOT IN (SELECT user_id FROM sidewalk_user);

Suggested query

DELETE wa
FROM webpage_activity wa
LEFT JOIN sidewalk_user su ON wa.user_id = su.user_id
WHERE wa.user_id IS NULL OR su.user_id IS NULL;

And then me editing it to fit more with our typical style:

DELETE
FROM webpage_activity
LEFT JOIN sidewalk_user ON webpage_activity.user_id = sidewalk_user.user_id
WHERE webpage_activity.user_id IS NULL OR sidewalk_user.user_id IS NULL;

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

We got here eventually 😁

@misaugstad misaugstad merged commit cabde7a into develop Dec 6, 2023
@misaugstad misaugstad deleted the 2091-constraint branch December 6, 2023 19:52
@misaugstad misaugstad mentioned this pull request Dec 7, 2023
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.

Add user_id constraint to webpage_activity table
2 participants