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

docs(pg-adapter): add unique and foreign key constraints to the PG adapter schema #12381

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

baptiste00001
Copy link

☕️ Reasoning

The contraints in the PG adapter schema are non existant while they are defined in all other adapters. I added them to the schema.

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

Copy link

vercel bot commented Dec 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 22, 2024 9:57am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Dec 22, 2024 9:57am

Copy link

vercel bot commented Dec 14, 2024

@baptiste00001 is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@ndom91 ndom91 left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@ndom91 ndom91 changed the title Add unique and foreign key constraints to the PG adapter schema docs(pg-adapter): add unique and foreign key constraints to the PG adapter schema Dec 19, 2024
@ndom91
Copy link
Member

ndom91 commented Dec 19, 2024

Ah wait, we may need to remove the unique constraint on email. Will that fail if we try to use null for multiple users email?

@baptiste00001
Copy link
Author

baptiste00001 commented Dec 19, 2024

Ah wait, we may need to remove the unique constraint on email. Will that fail if we try to use null for multiple users email?

It works fine with null value for multiple users email because there is no NULL NOT DISTINCT after the UNIQUE constraint.
So two null values are considered distinct.
I just tested it.

I think email should probably be unique in most cases, but feel free to remove the unique constraint if you think it fits the broader use cases.
In my use case I need to have email as unique so I added it.

@ndom91
Copy link
Member

ndom91 commented Dec 22, 2024

I approved the test run from fork to ensure the pg schema still works. It ran into a few prettier issues. I know one isn't from a file in your PR, but would you mind running prettier over these here? 🙏

See: https://github.com/nextauthjs/next-auth/actions/runs/12443333045/job/34755721579?pr=12381

EDIT: Neverind, fixed it here

Add unique and foreign key constraints to the schema
Add unique and foreign key contraints to the schema
user's email may be NULL
user's email may be NULL
For some reason cascade delete does not work without explicitly writing the primary key (id) in the foreign key definition.
For some reason cascade delete does not work without explicitly writing the primary key (id) in the foreign key definition.
@baptiste00001
Copy link
Author

I approved the test run from fork to ensure the pg schema still works. It ran into a few prettier issues. I know one isn't from a file in your PR, but would you mind running prettier over these here? 🙏

See: https://github.com/nextauthjs/next-auth/actions/runs/12443333045/job/34755721579?pr=12381

EDIT: Neverind, fixed it here

Oh OK
I actually fixed it as well and commited.
It was a bunch of extra spaces that needed to be removed in two files.
I hope you can merge the PR now.
If you need anything else just ask me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters Changes related to the core code concerning database adapters pg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants