[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Change by Dong-hee Na : -- stage: patch review -> resolved status: open -> closed ___ Python tracker ___ ___ Python-bugs-list

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Dong-hee Na added the comment: New changeset 38afeb1a336f0451c0db86df567ef726f49f6438 by Erlend Egeberg Aasland in branch 'main': bpo-46249: Move set lastrowid out of the sqlite3 query loop (GH-30489) https://github.com/python/cpython/commit/38afeb1a336f0451c0db86df567ef726f49f6438

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Marc-Andre Lemburg added the comment: On 11.01.2022 21:30, Erlend E. Aasland wrote: > >> I'd suggest to deprecate the cursor.lastrowid attribute and >> instead point people to the much more useful [...] > > Yes, I think mentioning the RETURNING ROWID trick in the sqlite3 docs is a > very

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Erlend E. Aasland added the comment: > You don't know on which cursor the last row was inserted [...] SQLite has no concept of cursors :) > It also seems that the function really only works for INSERTs and > not for UPDATEs. Yes, hence it's name: sqlite3_last_insert_rowid() ! > I'd suggest

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Marc-Andre Lemburg added the comment: On 11.01.2022 20:46, Erlend E. Aasland wrote: > > If we are to revert to this behaviour, we'll have to start examining the SQL > we are given (search for INSERT and REPLACE keywords, determine if they are > valid (i.e. not a comment, not part of a column

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Erlend E. Aasland added the comment: > OTOH, the SQLite API is tied to the _connection_ object, so it may not make > sense to align it with lastrowid which is a _cursor_ attribute. That came out weird; let's try again: The SQLite API is tied to the _connection_ object, so it may not make

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Erlend E. Aasland added the comment: > Is 0 a valid row ID in SQLite ? If not, then I guess this would be > an alternative to None as suggested by the DB-API. Yes. Any 64 bit signed integer value is a valid row id in SQLite. > If it is a valid row ID, I'd suggest to go back to resetting to

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Marc-Andre Lemburg added the comment: On 08.01.2022 21:56, Erlend E. Aasland wrote: > > Marc-André: since Python 3.6, the sqlite3.Cursor.lastrowid attribute does no > longer comply with the recommendations of PEP 249: > > Previously, lastrowid was set to None for operations other than

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Change by Erlend E. Aasland : -- pull_requests: +28692 pull_request: https://github.com/python/cpython/pull/30489 ___ Python tracker ___

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Erlend E. Aasland added the comment: I see now that GH-30380 has grown a little bit out of scope, as it changes too many things at once: 1. Simplify the `execute()`/`executemany()` query loop 2. Create the lastrowid PyObject on demand, simplifying cursor GC 3. `lastrowid` is now available

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Erlend E. Aasland added the comment: Marc-André: since Python 3.6, the sqlite3.Cursor.lastrowid attribute does no longer comply with the recommendations of PEP 249: Previously, lastrowid was set to None for operations other than INSERT or REPLACE. This changed with

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Erlend E. Aasland added the comment: There are some inaccuracies in the docs I need to fix first, since those changes must be backported, and I want the updated docs applied upon the corrected docs; I'll open a separate issue/PR for that. -- ___

[issue46249] [sqlite3] move set lastrowid out of the query loop and enable it for executemany()

Change by Erlend E. Aasland : -- title: [sqlite3] move set lastrowid out of the query loop -> [sqlite3] move set lastrowid out of the query loop and enable it for executemany() ___ Python tracker

[issue46249] [sqlite3] move set lastrowid out of the query loop

Erlend E. Aasland added the comment: True that :) I'll close GH-30371 and prepare GH-30380 for review. I'll request your review if you don't mind. -- ___ Python tracker ___

[issue46249] [sqlite3] move set lastrowid out of the query loop

Marc-Andre Lemburg added the comment: On 04.01.2022 21:02, Erlend E. Aasland wrote: > > Erlend E. Aasland added the comment: > >> If possible, it's usually better to have the .executemany() create a >> cursor with an output array providing the row ids, e.g. using "INSERT ... >> RETURNING

[issue46249] [sqlite3] move set lastrowid out of the query loop

Erlend E. Aasland added the comment: > If possible, it's usually better to have the .executemany() create a > cursor with an output array providing the row ids, e.g. using "INSERT ... > RETURNING ..." (PostgreSQL). That way you can access all row ids and > can also provide the needed detail in

[issue46249] [sqlite3] move set lastrowid out of the query loop

Marc-Andre Lemburg added the comment: On 04.01.2022 11:02, Erlend E. Aasland wrote: > > Erlend E. Aasland added the comment: > > Thank you for your input Marc-André. > > For SQLite, it's pretty simple: we use an API called > sqlite3_last_insert_rowid() which takes the database connection

[issue46249] [sqlite3] move set lastrowid out of the query loop

Erlend E. Aasland added the comment: Thank you for your input Marc-André. For SQLite, it's pretty simple: we use an API called sqlite3_last_insert_rowid() which takes the database connection as it's argument, not a statement pointer. This function returns "the rowid of the most recent

[issue46249] [sqlite3] move set lastrowid out of the query loop

Marc-Andre Lemburg added the comment: On 04.01.2022 00:49, Erlend E. Aasland wrote: > > Erlend E. Aasland added the comment: > > I see that PEP 249 does not define the semantics of lastrowid for > executemany(). What's the precedence here, MAL? IMO, it would be nice to be > able to fetch

[issue46249] [sqlite3] move set lastrowid out of the query loop

Erlend E. Aasland added the comment: > So, another option would be to keep "set-lastrowid" in the query loop, and > just remove the condition; we set it every time (but of course only build a > PyObject of it when the loop is done). I made a competing PR (GH-30380) for this, just a little

[issue46249] [sqlite3] move set lastrowid out of the query loop

Change by Erlend E. Aasland : -- pull_requests: +28590 pull_request: https://github.com/python/cpython/pull/30380 ___ Python tracker ___

[issue46249] [sqlite3] move set lastrowid out of the query loop

Erlend E. Aasland added the comment: I see that PEP 249 does not define the semantics of lastrowid for executemany(). What's the precedence here, MAL? IMO, it would be nice to be able to fetch the last row id after you've done a 1000 inserts using executemany(). So, another option would be

[issue46249] [sqlite3] move set lastrowid out of the query loop

Change by Erlend E. Aasland : -- keywords: +patch pull_requests: +28583 stage: -> patch review pull_request: https://github.com/python/cpython/pull/30371 ___ Python tracker

[issue46249] [sqlite3] move set lastrowid out of the query loop

New submission from Erlend E. Aasland : The query loop in _pysqlite_query_execute() is run only once for ordinary execute()'s, but multiple times for executemany(). We use the 'multiple' variable to differ between the two execute methods; for execute(), multiple is false, for executemany(),