prisma-engines
prisma-engines copied to clipboard
test: add TiDB into the engine tests
@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
@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
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.
We created a local branch that also runs the tests on Buildkite: https://github.com/prisma/prisma-engines/pull/3410 All ✅
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
That sounds like a great idea @Mini256.
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.
Remaining test failure seems to be a missing index @@index([pastryName], map: "pastryfk").
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'.
CodSpeed Performance Report
Merging #3308 will not alter performances
Comparing Mini256:fix-testes-for-tidb (fed4a9b) with main (934b869)
Summary
✅ 11 untouched benchmarks
(Nice work you are doing here right now @Mini256!)
@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_LOCKoperations during theapply_migrationstage, some tests may encounter errors due to lock conflicts.But this issue only occurs when
tidb-serveris running separately (usingmockkvto mock the underlying storage engineTiKV), 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 ofmockkvandTiKV, related issues: https://github.com/pingcap/tidb/issues/35155#issuecomment-1520428765
@janpio Updated the Docker image of TiDB to version 7.1.0. (Ready to merged) TiDB 7.1.0 Release Note
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.