prisma-engines icon indicating copy to clipboard operation
prisma-engines copied to clipboard

test: add TiDB into the engine tests

Open Mini256 opened this issue 3 years ago • 13 comments

part of #14639

Mini256 avatar Oct 20 '22 06:10 Mini256

@janpio, thanks for your reviewing! For the rest testcase, I need to make some investigatoin and responsd later. I will finish it ASAP.

cc: @AV25242, @zhangyangyu

Mini256 avatar Nov 10 '22 09:11 Mini256

@janpio, I think I have responded all the review comments, and if I can, I’ll push a commit to get the tests up and running in GitHub Action.

cc: @AV25242, @zhangyangyu

Mini256 avatar Nov 16 '22 15:11 Mini256

Can you maybe do a quick contribution (does not have to make sense, adding an empty line at the end) of one of our .md files? Then I can merge that and this PR here should automatically run GitHub actions.

janpio avatar Nov 16 '22 17:11 janpio

We created a local branch that also runs the tests on Buildkite: https://github.com/prisma/prisma-engines/pull/3410 All ✅

janpio avatar Nov 17 '22 15:11 janpio

We created a local branch that also runs the tests on Buildkite: https://github.com/prisma/prisma-engines/pull/3410 All ✅

@janpio Should I add TIDB to matrix in the GitHub Action?

https://github.com/prisma/prisma-engines/blob/f9e3461050264e663b646c4e52fdec94655f3a9c/.github/workflows/migration-engine.yml#L62

https://github.com/prisma/prisma-engines/blob/f9e3461050264e663b646c4e52fdec94655f3a9c/.github/workflows/query-engine.yml#L20

Mini256 avatar Nov 17 '22 16:11 Mini256

That sounds like a great idea @Mini256.

janpio avatar Nov 18 '22 13:11 janpio

Failed test on "Test mysql_5_6 on Linux"

---- identify_version::introspect_mysql_non_prisma stdout ----
thread 'identify_version::introspect_mysql_non_prisma' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: QueryError(Server(ServerError { code: 1064, message: "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'json)' at line 1", state: "42000" })), original_code: Some("1064"), original_message: Some("You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'json)' at line 1") }', introspection-engine/introspection-engine-tests/tests/identify_version/mod.rs:218:1

MySQL 5.6 does not support the json type, but supports the point type.

Mini256 avatar Dec 08 '22 16:12 Mini256

Remaining test failure seems to be a missing index @@index([pastryName], map: "pastryfk").

janpio avatar Dec 09 '22 02:12 janpio

Remaining test failure seems to be a missing index @@index([pastryName], map: "pastryfk").

Yep, I think we need to exclude this testcase first before TiDB Foreign Key feature GA.

Or we need to build a custom TiDB docker image to enable Foreign Key preview feature by SQL set global tidb_enable_foreign_key = 'ON'.

Mini256 avatar Dec 09 '22 02:12 Mini256

CodSpeed Performance Report

Merging #3308 will not alter performances

Comparing Mini256:fix-testes-for-tidb (fed4a9b) with main (934b869)

Summary

✅ 11 untouched benchmarks

codspeed-hq[bot] avatar Apr 20 '23 10:04 codspeed-hq[bot]

(Nice work you are doing here right now @Mini256!)

janpio avatar Apr 26 '23 10:04 janpio

@janpio Thank you for your attention to the pull request.

We hope to continue promoting this PR to merge, and after this PR is merged, our team will also continue to optimize the user experience of developers using Prisma with TiDB.

In the next stage, we will promote our TiDB Serverless product so that developers can use HTAP capabilities to build their applications with just MySQL skills (Just like how we used TiDB to build OSSInsight).


The code commit history may look a bit lengthy, so let me briefly summarize the recent changes I have made:

  • Updated the Docker image to version 7.0.0

  • Restart the tests skipped due to insufficient support for foreign keys previously

    But also I found some compatibility issues of naming, such as https://github.com/pingcap/tidb/issues/43267 (It doesn't prevent users from using related functions, but it may cause mismatches with some test case assertions. Currently, I am skipping these tests and will update them after fixing the issue.

  • Set the schema engine's testing to single-thread mode for TiDB

    The migration-engine-tests perform GET_LOCK operations during the apply_migration stage, some tests may encounter errors due to lock conflicts.

    But this issue only occurs when tidb-server is running separately (using mockkv to mock the underlying storage engine TiKV), and it can pass the test normally in a complete cluster, which means it will not affect user usage. I guess this may be due to some differences in the implementation of mockkv and TiKV, related issues: https://github.com/pingcap/tidb/issues/35155#issuecomment-1520428765

Mini256 avatar Apr 26 '23 13:04 Mini256

@janpio Updated the Docker image of TiDB to version 7.1.0. (Ready to merged) TiDB 7.1.0 Release Note

Mini256 avatar Jul 05 '23 10:07 Mini256

Thank you for submitting this PR, @Mini256!

We mentioned in our documentation that TiDB has a community-maintained driver adapter. This driver adapter already allows users to effortlessly query TiDB databases. Thus, we currently do not plan to merge this PR and https://github.com/prisma/prisma/pull/14631.

laplab avatar Apr 11 '24 12:04 laplab