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.

Sign up for GitHub

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

Jump to bottom

Add postgresql adapter for Kysely #307

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

Add postgresql adapter for Kysely #307

wants to merge 2 commits into from

Conversation

savoiringfaire
Copy link

@savoiringfaire savoiringfaire commented Jul 25, 2023 •
edited

The original problem I had which triggered this change was this error when running migrations as documented in https://kysely.dev/docs/migrations

VercelPostgresKyselyError [VercelPostgresError]: VercelPostgresError - 'kysely_transactions_not_supported': Transactions are not supported yet.

Which we get because we haven't properly signaled to Kysely that we don't support transactions. The Kysely migrator checks supportsTransactionalDdl to decide whether to run migrations in a transaction or not, so this adds that.

It also overrides the default migration lock functions from https://github.com/kysely-org/kysely/blob/master/src/dialect/postgres/postgres-adapter.ts to use pg_advisory_lock and then unlock with pg_advisory_unlock since we can't rely on transactions auto-dropping now.

Fixes #325

@changeset-bot
Copy link

changeset-bot bot commented Jul 25, 2023 •
edited

⚠️ No Changeset found

Latest commit: e5209bb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jul 25, 2023

@marcus-kepler is attempting to deploy a commit to the Curated Tests Team on Vercel.

A member of the Team first needs to authorize it.

@savoiringfaire savoiringfaire marked this pull request as draft July 25, 2023 21:01
@savoiringfaire
Copy link
Author

Tests on this currently failing due to lack of kysely-org/kysely#456 being officially released.

@savoiringfaire
Copy link
Author

Have un-extended the postgresql adapter and implemented the single remaining method in 20d17be which has got tests passing.

@savoiringfaire savoiringfaire marked this pull request as ready for review July 25, 2023 21:09
@marcus-kepler @savoiringfaire
Add postgresql adapter
ac9f7cd
@marcus-kepler @savoiringfaire
Don't extend postgresql adapter
e5209bb 
We can't extend the postgresql adapter until
kysely-org/kysely#456 is released, as it's not
currently fully implementing the interface. Instead copy the single
remaining member over for the time being.
volnei
volnei reviewed Aug 12, 2023
View reviewed changes
Copy link

@volnei volnei left a comment

Choose a reason for hiding this comment

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

Looks great

@volnei volnei mentioned this pull request Aug 12, 2023
Open
@volnei
Copy link

volnei commented Aug 15, 2023 •
edited

Can someone explain what I can do to have this working?

@vvo
Copy link
Member

vvo commented Nov 2, 2023

Hey there, getting back to this. We're investigating how much value our Kysely plugin provides versus a community one: https://github.com/seveibar/kysely-neon. Have any of you tried kysely-neon? We're wondering if we should deprecate our own adapter and delegate it to the community.

As you can tell, we have not been very active in this issue (apologies), which is why we're wondering if it should be owned by the community.

Let us know if https://github.com/seveibar/kysely-neon could work for you, and if not I can re-review this PR.

@aleksrutins aleksrutins mentioned this pull request Nov 5, 2023
Closed
@volnei
Copy link

volnei commented Nov 7, 2023

Hey there, getting back to this. We're investigating how much value our Kysely plugin provides versus a community one: https://github.com/seveibar/kysely-neon. Have any of you tried kysely-neon? We're wondering if we should deprecate our own adapter and delegate it to the community.

As you can tell, we have not been very active in this issue (apologies), which is why we're wondering if it should be owned by the community.

Let us know if https://github.com/seveibar/kysely-neon could work for you, and if not I can re-review this PR.

I agree with you since the kysely neon is an active project and cover all behaviours that vercel kysely do. Can we ensure that? Otherwise I prefer to mantain current with this fixes that I think that is the only problem.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@volnei volnei volnei left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
storage: postgres
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for kysely migration
5 participants
@savoiringfaire @volnei @vvo @dferber90 @marcus-kepler