classification
Title: sqlite3_column_bytes() should be called after sqlite3_column_blob()
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, erlendaasland, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2021-02-18 11:14 by erlendaasland, last changed 2021-02-18 17:13 by berker.peksag. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 24562 merged erlendaasland, 2021-02-18 11:39
PR 24565 merged erlendaasland, 2021-02-18 16:38
Messages (5)
msg387212 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-02-18 11:14
sqlite3_column_bytes() should be called _after_ sqlite3_column_blob(). There two calls to sqlite3_column_blob() are both preceeded by calls to sqlite3_column_bytes(). Currently it does not do any harm, but it is bad API usage.

I suggest to fix the two cases, and add a comment in the source code as a reminder.

Berker?


Quoting from https://sqlite.org/c3ref/column_blob.html:
The safest policy is to invoke these routines in one of the following ways:
  * sqlite3_column_text() followed by sqlite3_column_bytes()
  * sqlite3_column_blob() followed by sqlite3_column_bytes()
  * sqlite3_column_text16() followed by sqlite3_column_bytes16()

In other words, you should call sqlite3_column_text(), sqlite3_column_blob(), or sqlite3_column_text16() first to force the result into the desired format, then invoke sqlite3_column_bytes() or sqlite3_column_bytes16() to find the size of the result.
msg387216 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2021-02-18 11:24
Good catch! Sounds good to me but I'd say designing APIs relying on call order is bad too :)
msg387221 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-02-18 11:29
> Sounds good to me but I'd say designing APIs relying on call order is bad too :)

Yes, it's too easy to mess up things with this part of the SQLite API :) I'll throw up a PR.


BTW (small digression):
I really want to refactor _pysqlite_fetch_one_row() so it uses two helpers (for example _fetch_nth_column_with_converter() and _fetch_nth_column()). It will be easier to maintain, and it's way easier to read. The coverage for this function is close to 100%, so it should be pretty safe to refactor it.
msg387241 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2021-02-18 15:45
New changeset 47feb1feb28631b6647699b7633109aa85340966 by Erlend Egeberg Aasland in branch 'master':
bpo-43249: sqlite3_column_bytes() must follow sqlite_column_blob() (GH-24562)
https://github.com/python/cpython/commit/47feb1feb28631b6647699b7633109aa85340966
msg387247 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2021-02-18 17:13
New changeset cc96231f0a59cc7393943064800ecb6c18892662 by Erlend Egeberg Aasland in branch 'master':
bpo-43249: Improve scoping in _pysqlite_fetch_one_row() (GH-24565)
https://github.com/python/cpython/commit/cc96231f0a59cc7393943064800ecb6c18892662
History
Date User Action Args
2021-02-18 17:13:44berker.peksagsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-02-18 17:13:23berker.peksagsetmessages: + msg387247
2021-02-18 16:38:04erlendaaslandsetpull_requests: + pull_request23346
2021-02-18 15:45:03berker.peksagsetmessages: + msg387241
2021-02-18 11:39:36erlendaaslandsetkeywords: + patch
stage: patch review
pull_requests: + pull_request23343
2021-02-18 11:29:42erlendaaslandsetmessages: + msg387221
2021-02-18 11:24:02berker.peksagsetmessages: + msg387216
2021-02-18 11:14:23erlendaaslandcreate