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

Improve ORM Compatibility #24194

Open
4 of 11 tasks
zz-jason opened this issue Apr 21, 2021 · 22 comments
Open
4 of 11 tasks

Improve ORM Compatibility #24194

zz-jason opened this issue Apr 21, 2021 · 22 comments
Assignees

Comments

@zz-jason
Copy link
Member

zz-jason commented Apr 21, 2021

Introduction

ORM, which is short for Object-Relational Mapping, is usually used by applications to connect to databases like TiDB.

In order to bring good experience to application developers and enrich the TiDB ecosystem, let's:

  1. investigate the popular ORM frameworks for different programming language
  2. fix the compatibility issues between TiDB and these ORM frameworks

Candidates

You may comment on this issue about the ORMs that you are using.

Goal

Considering TiDB is not 100% compatible with MySQL with:

We aim to go through the different ORMs listed above and provide the built-in implementation for TiDB.

Reference

@zz-jason zz-jason added type/feature-request Categorizes issue or PR as related to a new feature. and removed type/feature-request Categorizes issue or PR as related to a new feature. labels Apr 21, 2021
@ichn-hu
Copy link
Contributor

ichn-hu commented Apr 25, 2021

Shall we consider TypeORM (23k star), Sequelize (24k star) and Prisma (10k star) from the Nodejs community? IMO they are growing very rapidly in the oversea development community.

@dveeden
Copy link
Contributor

dveeden commented May 5, 2021

I had a quick test with Django 3.2.1 (Python).

I created a new project and app and configured the database config. Then I added the models from the tutorial an ran migrations etc.

With the 'admin' site I added and removed choices and questions. This seemed to work.
When deleting a question while there are choices that have a foreign key to that question this was working fine as Django was smart enough to process that itself. However manually removing a question left the database in an inconsistent state where there were choices that referred to nonexistent questions.

So the basic functionality seems to work, but like many other ORMs it would be good to add full support for foreign keys.

@zz-jason
Copy link
Member Author

zz-jason commented May 5, 2021

@dveeden Thanks for your reminder! Seems we have compatibility issues with Django as well, I'll add it to the list. BTW, are you interested in contributing to this project?

@zz-jason
Copy link
Member Author

zz-jason commented May 5, 2021

Shall we consider TypeORM (23k star), Sequelize (24k star) and Prisma (10k star) from the Nodejs community? IMO they are growing very rapidly in the oversea development community.

@ichn-hu Good idea! Added to the list as well.

@dveeden
Copy link
Contributor

dveeden commented May 5, 2021

@dveeden Thanks for your reminder! Seems we have compatibility issues with Django as well, I'll add it to the list. BTW, are you interested in contributing to this project?

Yes.

@moyun
Copy link

moyun commented May 10, 2021

for JAVA, it seems integration with libraries like Apache MetaModel, DataNucleus will bring great flexibility and generalizability to the data access layer.

@dveeden
Copy link
Contributor

dveeden commented Jun 23, 2021

@dveeden
Copy link
Contributor

dveeden commented Jun 23, 2021

Running pytest --dburi mysql://root@127.0.0.1:4000/test with sqlalchemy master (2f100a7d4bb143e1f8674a388d8d305be3051bd6) and TiDB nightly (5.7.25-TiDB-v5.2.0-alpha-88-ged52601e6) results in:

=========================== short test summary info ============================
SKIPPED [47] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'MypyPluginTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'custom function
SKIPPED [5] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'AliasedClassRelationshipTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'Backend does not support window functions
SKIPPED [6] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'AsyncPgTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'ConcurrentAutomapTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'
SKIPPED [5] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: No profiling stats available on this platform for this function.  Run tests with --write-profiles to add statistics to profiles.txt for this platform.
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'test/dialect/postgresql/test_compiler.py::SequenceTest::test_reverse_eng_name (call)' : foo
FAILED test/orm/declarative/test_basic.py::DeclarativeTest_dynamic::test_with_explicit_autoloaded
FAILED test/orm/declarative/test_basic.py::DeclarativeTest_explicit::test_with_explicit_autoloaded
FAILED test/orm/declarative/test_reflection.py::DeclarativeReflectionTest::test_basic
FAILED test/orm/declarative/test_reflection.py::DeclarativeReflectionTest::test_rekey_wbase
FAILED test/orm/declarative/test_reflection.py::DeclarativeReflectionTest::test_rekey_wdecorator
FAILED test/orm/declarative/test_reflection.py::DeclarativeReflectionTest::test_supplied_fk
FAILED test/orm/inheritance/test_assorted_poly.py::RelationshipTest5::test_eager_empty
FAILED test/ext/test_automap.py::AutomapInhTest::test_conditional_relationship
FAILED test/ext/test_automap.py::AutomapInhTest::test_joined_inheritance_reflect
FAILED test/ext/test_automap.py::AutomapInhTest::test_single_inheritance_reflect
FAILED test/ext/test_automap.py::AutomapTest::test_prepare_w_only - sqlalchem...
FAILED test/orm/test_bundle.py::BundleTest::test_bundle_nesting_unions - Asse...
FAILED test/base/test_concurrency_py3k.py::TestAsyncAdaptedQueue::test_error_other_loop
FAILED test/base/test_concurrency_py3k.py::TestAsyncAdaptedQueue::test_lazy_init
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_async_error
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_await_fallback_error
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_await_only_error
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_await_only_no_greenlet
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_contextvars
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_ok - Value...
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_propagate_cancelled
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_require_await
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_sync_error
FAILED test/sql/test_constraints.py::ConstraintGenTest_mysql+mysqldb_5_7_25_2_0_88::test_cycle_unnamed_fks
FAILED test/sql/test_constraints.py::ConstraintGenTest_mysql+mysqldb_5_7_25_2_0_88::test_fk_cant_drop_cycled_unnamed
============= 25 failed, 2275 passed, 65 skipped in 754.42 seconds =============

Looks like the issues are:

@dveeden
Copy link
Contributor

dveeden commented Jun 23, 2021

With sqlalchemy/sqlalchemy#6660 this changes to:

=========================== short test summary info ============================
SKIPPED [47] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'MypyPluginTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'custom function
SKIPPED [5] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'AliasedClassRelationshipTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'Backend does not support window functions
SKIPPED [6] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'AsyncPgTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'ConcurrentAutomapTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'
SKIPPED [5] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: No profiling stats available on this platform for this function.  Run tests with --write-profiles to add statistics to profiles.txt for this platform.
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'test/dialect/postgresql/test_compiler.py::SequenceTest::test_reverse_eng_name (call)' : foo
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'test/sql/test_defaults.py::AutoIncrementTest_mysql+mysqldb_5_7_25_2_0_88::test_non_autoincrement (call)' : not sqlite
SKIPPED [5] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'CTEDefaultTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'custom function and not mariadb > (10, 2) and not postgresql and not mssql and not oracle and not sqlite >= (3, 8, 3)
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'IdentityDefaultsOnUpdateTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'not postgresql >= (10,) and not oracle >= (12,) and not mssql
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'test/sql/test_defaults.py::PKDefaultTest_mysql+mysqldb_5_7_25_2_0_88::test_with_implicit_returning (call)' : mysql doesn't support 'RETURNING of a single row'
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'test/sql/test_defaults.py::ServerDefaultsOnPKTest_mysql+mysqldb_5_7_25_2_0_88::test_int_default_on_insert_with_returning (call)' : mysql doesn't support 'RETURNING of a single row'
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'test/sql/test_defaults.py::ServerDefaultsOnPKTest_mysql+mysqldb_5_7_25_2_0_88::test_string_default_on_insert_with_returning (call)' : mysql doesn't support 'RETURNING of a single row'
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'test/sql/test_defaults.py::SpecialTypePKTest_mysql+mysqldb_5_7_25_2_0_88::test_no_implicit_returning (call)' : mysql doesn't support 'RETURNING of a single row'
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'test/sql/test_defaults.py::SpecialTypePKTest_mysql+mysqldb_5_7_25_2_0_88::test_server_default_no_implicit_returning (call)' : mysql doesn't support 'RETURNING of a single row'
SKIPPED [2] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'TriggerDefaultsTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'requires SUPER priv or requires SUPER priv or not supported by database
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'AutocommitClosesOnFailTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'not oracle and not postgresql
SKIPPED [1] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'test/sql/test_deprecations.py::ConnectionlessCursorResultTest_mysql+mysqldb_5_7_25_2_0_88::test_connectionless_autoclose_crud_rows_exhausted (call)' : mysql doesn't support 'RETURNING of a single row'
SKIPPED [5] /home/dvaneeden/dev/sqlalchemy/test/../lib/sqlalchemy/testing/config.py:206: 'ExplicitAutoCommitTest' unsupported on any DB implementation 'mysql(5.7.25.2.0.88)+mysqldb'
FAILED test/orm/inheritance/test_assorted_poly.py::RelationshipTest5::test_eager_empty
FAILED test/orm/test_bundle.py::BundleTest::test_bundle_nesting_unions - Asse...
FAILED test/base/test_concurrency_py3k.py::TestAsyncAdaptedQueue::test_error_other_loop
FAILED test/base/test_concurrency_py3k.py::TestAsyncAdaptedQueue::test_lazy_init
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_async_error
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_await_fallback_error
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_await_only_error
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_await_only_no_greenlet
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_contextvars
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_ok - Value...
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_propagate_cancelled
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_require_await
FAILED test/base/test_concurrency_py3k.py::TestAsyncioCompat::test_sync_error
FAILED test/sql/test_constraints.py::ConstraintGenTest_mysql+mysqldb_5_7_25_2_0_88::test_cycle_unnamed_fks
FAILED test/sql/test_constraints.py::ConstraintGenTest_mysql+mysqldb_5_7_25_2_0_88::test_fk_cant_drop_cycled_unnamed
FAILED test/sql/test_constraints.py::ConstraintGenTest_mysql+mysqldb_5_7_25_2_0_88::test_fk_column_auto_alter_inline_constraint_create
FAILED test/sql/test_constraints.py::ConstraintGenTest_mysql+mysqldb_5_7_25_2_0_88::test_fk_column_use_alter_constraint_create
FAILED test/sql/test_constraints.py::ConstraintGenTest_mysql+mysqldb_5_7_25_2_0_88::test_fk_column_use_alter_inline_constraint_create
FAILED test/sql/test_constraints.py::ConstraintGenTest_mysql+mysqldb_5_7_25_2_0_88::test_fk_table_auto_alter_constraint_create
FAILED test/sql/test_constraints.py::ConstraintGenTest_mysql+mysqldb_5_7_25_2_0_88::test_fk_table_use_alter_constraint_create
FAILED test/orm/test_cycles.py::InheritTestTwo::test_flush - sqlalchemy.exc.O...
======== 21 failed, 3012 passed, 86 skipped, 4 error in 1010.98 seconds ========

Note that the number of failed tests decreased by 4 and the number of passed tests increased by 737. The number of skipped tests also increased.

@dragonly
Copy link
Contributor

dragonly commented Jul 8, 2021

IMHO, we should remove those (to be listed) check boxes, because they're semantically unnecessary, and misleads the task counter. @zz-jason

@hooopo
Copy link

hooopo commented Aug 12, 2021

activerecord-tidb-adapter is ready, and this is demo app using activerecord-tidb-adapter https://github.com/hooopo/activerecord-tidb-adapter-demo

@Windforce17
Copy link

set DisableNestedTransaction: true when call gorm.Open,and gorm won't use savepoint.

@Pythoner6
Copy link

Pythoner6 commented Dec 4, 2022

I recently tried using tidb as a backend for https://github.com/dani-garcia/vaultwarden and it almost works, but something is funky about the migrations it runs on startup. It's using diesel and runs through these migrations: https://github.com/dani-garcia/vaultwarden/tree/main/migrations/mysql, but something goes wrong with the migrations that causes it to spit out things like this (this log is from reproducing it with the diesel cli, but it's the same issue running the application):

Running migration 2018-01-14-171611_create_tables
Running migration 2018-02-17-205753_create_collections_and_orgs
Running migration 2018-04-27-155151_create_users_ciphers
Failed to run 2018-04-27-155151_create_users_ciphers with: Table 'vaultwarden.oldCiphers' doesn't exist

If I manually run the migration using the mariadb client, e.g. like this:

mariadb <connection parameters> vaultwarden -e "begin; $(cat 2018-04-27-155151_create_users_ciphers/up.sql) commit;"

So there's something that diesel is doing differently from the mariadb client when running the migration, but diesel doesn't seem to have any logging about exactly what it's doing which makes it hard to figure out what's going on here. I assume it has something to do with renaming and then referencing a table within the same transaction or something perhaps? But I really don't know enough about either diesel or TiDB.

@Pythoner6
Copy link

Pythoner6 commented Dec 4, 2022

Aha, I think I figured it out. TiDB seems to behave differently than MySQL when dealing with multi-statement queries, which diesel is using for the migrtions - it looks like it makes one call to submit the whole migration as one query, vs the mariadb cli which seems to split each statement out into a separate query. When I run the migration in the mariadb cli after changing the delimiter I'm able to reproduce the problem there. I don't see any thing about multi-statement queries listed in the sections about compatibility though, is this a known difference?

@dveeden
Copy link
Contributor

dveeden commented Dec 5, 2022

@Pythoner6 what TiDB version did you try?

Older versions of TiDB didn't support ALTER statements with multiple operations. Maybe this is what's causing this?

@Pythoner6
Copy link

Pythoner6 commented Dec 5, 2022

I dont remember right now but it should have been the latest version supported by the operator which i installed via helm. But I dont think that's it, unless I'misunderstanding what you mean. The query that fails looks like this:

ALTER TABLE ciphers RENAME TO oldCiphers;

CREATE TABLE ciphers (
  ...
);

...

INSERT INTO ciphers (uuid, created_at, updated_at, user_uuid, organization_uuid, type, name, notes, fields, data, favorite) 
SELECT uuid, created_at, updated_at, user_uuid, organization_uuid, type, name, notes, fields, data, favorite FROM oldCiphers;
...

(Full query here https://github.com/dani-garcia/vaultwarden/blob/main/migrations/mysql/2018-04-27-155151_create_users_ciphers/up.sql)

The alter statement is a rename, and the thing that's failing is an insert into from the renamed table. When diesel executes this, it makes a single call to mysql_real_query, and that fails with the table doesn't exist error. But I believe by default when I run things using the e.g. mariadb cli it parses the query and splits it into the individual statements and submits each as a separate query. This works fine.

@dveeden
Copy link
Contributor

dveeden commented Dec 6, 2022

That certainly looks like something that should work with TiDB, including older versions.

Is there anything in the logs of the TiDB nodes about this?

Would it be possible for you to try this with a DO SLEEP(5) just before the INSERT..SELECT...?

Is there an easy way for me to reproduce this? I assume I could build vaultwarden from source and then run it against a DB. Do I first need to use an older version to trigger the migration?

@Pythoner6
Copy link

I'm not super familiar with tidb, so maybe I'm missing something but I couldn't find any useful logs (though also the dashboard seemed to be failing to load logs in general so that may have been the issue). To reproduce the issue without running vaultwarden, I just installed the diesel cli, checked out the vaultwarden repo, navigated to the mysql migrations folder I linked, and ran diesel migration run --migration dir . --database-url .... I was also able to replicate it by manually using the mariadb cli running each up.sql of the migrations in turn after setting a different delimiter, like ;;;. I'll see if I can make a minimal replication of the issue too though.

@dveeden
Copy link
Contributor

dveeden commented Dec 6, 2022

I wasn't able to reproduce this with a different delimiter. This is with MySQL Shell 8.0 as client.

sql> set tidb_multi_statement_mode='ON';
Query OK, 0 rows affected (0.0004 sec)

sql> CREATE TABLE t1(id int primary key);
Query OK, 0 rows affected (0.1348 sec)

sql> INSERT INTO t1 VALUES (1),(2),(3);
Query OK, 3 rows affected (0.0124 sec)

Records: 3  Duplicates: 0  Warnings: 0

sql> delimiter $$

sql> ALTER TABLE t1 RENAME TO t2; CREATE TABLE t1(id int primary key); INSERT INTO t1 SELECT * FROM t2; $$
Query OK, 0 rows affected (0.1336 sec)

Query OK, 0 rows affected (0.1336 sec)

Query OK, 0 rows affected (0.1336 sec)

sql> delimiter ;

sql> SELECT tidb_version()\G
*************************** 1. row ***************************
tidb_version(): Release Version: v6.4.0
Edition: Community
Git Commit Hash: cf36a9ce2fe1039db3cf3444d51930b887df18a1
Git Branch: heads/refs/tags/v6.4.0
UTC Build Time: 2022-11-13 05:25:30
GoVersion: go1.19.2
Race Enabled: false
TiKV Min Version: 6.2.0-alpha
Check Table Before Drop: false
Store: tikv
1 row in set (0.0004 sec)

@Pythoner6
Copy link

Ah, it seems this happens only if that multi-statement query is inside a transaction (which is how diesel runs migrations). Here is the minimal reproduction:

mariadb --host=127.0.0.1 --port=4000 --user=root --database test --delimiter ';;;' --execute "$(cat << EOF
-- First migration creates the table foo
BEGIN;
;;;
CREATE TABLE foo(pk CHAR(1) PRIMARY KEY);
;;;
COMMIT;
-- Second migration renames foo, creates a new foo, 
-- and selects into the new foo from the old foo
;;;
BEGIN;
;;;
ALTER TABLE foo RENAME TO bar; 
CREATE TABLE foo(pk CHAR(1) PRIMARY KEY);
INSERT INTO foo (pk) SELECT pk FROM bar;
;;;
COMMIT;
;;;
EOF
)"

Tested against a new instance run by tiup:

*************************** 1. row ***************************
tidb_version(): Release Version: v6.4.0
Edition: Community
Git Commit Hash: cf36a9ce2fe1039db3cf3444d51930b887df18a1
Git Branch: heads/refs/tags/v6.4.0
UTC Build Time: 2022-11-13 05:25:30
GoVersion: go1.19.2
Race Enabled: false
TiKV Min Version: 6.2.0-alpha
Check Table Before Drop: false
Store: tikv

@dveeden
Copy link
Contributor

dveeden commented Dec 6, 2022

Thanks. I was able to reproduce the issue. I've created #39664 for this.

Note that many DDL statements in MySQL (and MySQL compatible) databases do an implicit commit.

https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html

But with that this should still work.

@dveeden
Copy link
Contributor

dveeden commented Mar 7, 2023

Note that Foreign Keys are now supported since v6.6.0 which makes supporting ORMs a lot easier

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

No branches or pull requests

9 participants