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

Add support for MySQL #7538

Merged
merged 29 commits into from
Jun 14, 2022
Merged

Add support for MySQL #7538

merged 29 commits into from
Jun 14, 2022

Conversation

borisno2
Copy link
Member

@borisno2 borisno2 commented May 16, 2022

Adding support for MySQL

Allows the setting of db.provider to mysql in keystone.ts config.

URL for MySQL as per Prisma connection details

Things to keep in mind when using MySQL - doc page added to cover this as well:

  • When using autoincrement MySQL requires the field to also be indexed - added check to ensure this is set when using MySQL.
  • Added tests for mysql to match current postgresql tests
  • MySQL is not case sensitive - currently, the postgres tests are case sensitive and sqlite skips the string tests which is also what this current PR does for mysql - this is not a blocker for this PR, however, an issue is logged - Un-skip filter tests #7574 - to complete this work. Also adding documentation
  • Prisma string uses varchar(191) in MySQL instead of text in Postgres - added nativeType to text field so this can be easily overridden.

@changeset-bot
Copy link

changeset-bot bot commented May 16, 2022

🦋 Changeset detected

Latest commit: 3a80a4c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages
Name Type
@keystone-6/cloudinary Major
@keystone-6/core Minor
@keystone-6/auth Major
@keystone-6/fields-document Major
@keystone-6/session-store-redis Major
@keystone-6/example-auth Patch
@keystone-6/examples-app-basic Patch
@keystone-6/example-ecommerce Patch
@keystone-6/example-graphql-api-endpoint Patch
@keystone-6/example-roles Patch
@keystone-6/example-testing Patch
@keystone-6/example-with-auth Patch
@keystone-6/sandbox Patch
@keystone-6/website Patch
@keystone-6/example-document-field Patch

Not sure what this means? Click here to learn what changesets are.

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

@vercel
Copy link

vercel bot commented May 16, 2022

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

Name Status Preview Updated
keystone-next-docs ✅ Ready (Inspect) Visit Preview Jun 14, 2022 at 5:12AM (UTC)

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 16, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3a80a4c:

Sandbox Source
@keystone-6/sandbox Configuration

@vercel vercel bot temporarily deployed to Preview May 16, 2022 06:37 Inactive
@vercel vercel bot temporarily deployed to Preview May 16, 2022 06:46 Inactive
@vercel vercel bot temporarily deployed to Preview May 16, 2022 06:51 Inactive
@vercel vercel bot temporarily deployed to Preview May 16, 2022 07:09 Inactive
@vercel vercel bot temporarily deployed to Preview May 16, 2022 07:56 Inactive
@vercel vercel bot temporarily deployed to Preview May 16, 2022 22:03 Inactive
@vercel vercel bot temporarily deployed to Preview May 17, 2022 09:20 Inactive
@vercel vercel bot temporarily deployed to Preview May 17, 2022 10:15 Inactive
@dcousens dcousens changed the title Add support for mysql Add support for MySQL May 18, 2022
@emmatown
Copy link
Member

Prisma error different tests/api-tests/fields/unique.test.ts

For this, we should expect whatever it returns based on the database we're using. (Potentially in the future, we might want to have a consistent thing rather than what Prisma gives us but that is a totally separate thing to supporting MySQL)

Too many keys specified; max 64 keys allowed in tests/api-tests/access-control/schema.test.ts

I'm guessing that's because those tests create a huge number of lists. The fix here is probably going be to have a bunch of keystone instances with 1 list(or however many is convenient for how the tests are written) vs just a single keystone instance with a bunch of lists

@dcousens dcousens self-assigned this May 30, 2022
@vercel vercel bot temporarily deployed to Preview May 30, 2022 01:34 Inactive
@vercel vercel bot temporarily deployed to Preview May 30, 2022 01:37 Inactive
@emmatown emmatown self-assigned this May 30, 2022
@vercel vercel bot temporarily deployed to Preview May 30, 2022 01:43 Inactive
@vercel vercel bot temporarily deployed to Preview May 30, 2022 01:47 Inactive
@vercel vercel bot temporarily deployed to Preview May 30, 2022 04:51 Inactive
@vercel vercel bot temporarily deployed to Preview May 30, 2022 22:47 Inactive
Copy link
Contributor

@raveling raveling left a comment

Choose a reason for hiding this comment

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

looking great!
a few small suggestions for the guide

docs/components/docs/Navigation.tsx Outdated Show resolved Hide resolved

# Choosing the right Database

Keystone supports Postgres, MySQL and SQLite as database engines for your backend. There might be different reasons why you would choose one over the other.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we link to something for Postgres, MySQL, SQLite?

Perhaps other parts of our documentation that show how to configure each? 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

Added some links...

docs/pages/docs/guides/choosing-a-database.mdx Outdated Show resolved Hide resolved
docs/pages/docs/guides/choosing-a-database.mdx Outdated Show resolved Hide resolved
- [Database Features](https://www.prisma.io/docs/reference/database-reference/database-features)
- [Supported Databases](https://www.prisma.io/docs/reference/database-reference/supported-databases)

!> **Note**: Keystone currently supports Postgres, MySQL, SQLite
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure we need this due to the presence in the top of page callout.

If you'd like to keep it, suggest including Prisma somewhere to situate it in the Prisma H2 in a more obvious way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @raveling I have just rewritten this section

docs/pages/docs/guides/choosing-a-database.mdx Outdated Show resolved Hide resolved
docs/pages/docs/guides/choosing-a-database.mdx Outdated Show resolved Hide resolved
docs/pages/docs/guides/choosing-a-database.mdx Outdated Show resolved Hide resolved
Docs update

Co-authored-by: Ronald Aveling <luma.chroma@gmail.com>
@vercel vercel bot temporarily deployed to Preview May 31, 2022 09:57 Inactive
@vercel vercel bot temporarily deployed to Preview May 31, 2022 10:19 Inactive
@vercel vercel bot temporarily deployed to Preview May 31, 2022 23:18 Inactive
@vercel vercel bot temporarily deployed to Preview May 31, 2022 23:42 Inactive
@vercel vercel bot temporarily deployed to Preview May 31, 2022 23:56 Inactive
@dcousens dcousens assigned borisno2 and emmatown and unassigned dcousens, borisno2 and emmatown Jun 9, 2022
@borisno2 borisno2 marked this pull request as ready for review June 14, 2022 02:09
@vercel vercel bot temporarily deployed to Preview June 14, 2022 05:06 Inactive
@emmatown emmatown requested a review from raveling June 14, 2022 05:11
@vercel vercel bot temporarily deployed to Preview June 14, 2022 05:12 Inactive
@borisno2 borisno2 merged commit c862e87 into main Jun 14, 2022
@borisno2 borisno2 deleted the mysql branch June 14, 2022 22:38
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.

None yet

4 participants