tortoise / tortoise-orm

Familiar asyncio ORM for python, built with relations in mind
https://tortoise.github.io
Apache License 2.0
4.67k stars 392 forks source link

Incorrect return value with tconn.execute_query('insert') #540

Open fy0 opened 4 years ago

fy0 commented 4 years ago

Describe the bug Execute raw sql insert, result is incorrect.

To Reproduce

from tortoise import Tortoise, fields, run_async
from tortoise.models import Model

class Event(Model):
    id = fields.IntField(pk=True)
    name = fields.TextField()

    class Meta:
        table = "event"

    def __str__(self):
        return self.name

async def run():
    # await Tortoise.init(db_url="sqlite://:memory:", modules={"models": ["__main__"]})
    await Tortoise.init(db_url="postgres://user:pass@127.0.0.1:5432/example", modules={"models": ["__main__"]})
    await Tortoise.generate_schemas()

    from tortoise.transactions import in_transaction
    try:
        async with in_transaction() as tconn:
            r = await tconn.execute_query("insert into event values (10, '2')")
            print(r)

            print(await tconn.execute_query('select * from event'))
    except Exception as e:
        print(e)
        await tconn.rollback()

if __name__ == "__main__":
    run_async(run())

# You got this:
# (0, [])
# (6, [<Record id=1 name='2'>])

# Sqlite got this:
(1, [])
(1, [<sqlite3.Row object at 0x00000208DDA86ED0>])

Expected behavior (1, [])

Additional context It would be better to return lastrowid

long2ice commented 4 years ago

Did you try pure asyncpg? Because tortoise use it in deep.

DrJackilD commented 4 years ago

@fy0 I can't reproduce this bug with your example:

Output:

(0, [])
(1, [<Record id=10 name='2'>])
DrJackilD commented 4 years ago

@fy0 Actually, I think I've got what do you mean. With AsyncpgClient we return the length of the rows, which returned from the query, and asyncpg didn't return anything in the response on INSERT operation. On the other hand, SqliteClient returns (connection.total_changes - start) or len(rows) so or count of changes, or length of rows in a query. I believe with asyncpg we don't have such information like total changes. So, it's not really a lastrowid returned.

DrJackilD commented 4 years ago

Maybe could be better to have similar signatures for results in all clients? What do you think @long2ice ?

fy0 commented 4 years ago

@fy0 Actually, I think I've got what do you mean. With AsyncpgClient we return the length of the rows, which returned from the query, and asyncpg didn't return anything in the response on INSERT operation. On the other hand, SqliteClient returns (connection.total_changes - start) or len(rows) so or count of changes, or length of rows in a query. I believe with asyncpg we don't have such information like total changes. So, it's not really a lastrowid returned.

Right, sqlite have a lastrowid api, It is said that MySQL have a similar one. To postgres, you can append 'returning {primary_key}' after insert statement.