You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
sqlite3_column_bytes() should be called after sqlite3_column_blob() #87415
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee=Noneclosed_at=<Date2021-02-18.17:13:44.996>created_at=<Date2021-02-18.11:14:23.860>labels= ['type-feature', 'library', '3.10']
title='sqlite3_column_bytes() should be called after sqlite3_column_blob()'updated_at=<Date2021-03-02.12:18:11.676>user='https://github.com/erlend-aasland'
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.
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.
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.
New changeset 47feb1f by Erlend Egeberg Aasland in branch 'master': bpo-43249: sqlite3_column_bytes() must follow sqlite_column_blob() (GH-24562) 47feb1f
Activity
erlend-aasland commentedon Feb 18, 2021
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:
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.
berkerpeksag commentedon Feb 18, 2021
Good catch! Sounds good to me but I'd say designing APIs relying on call order is bad too :)
erlend-aasland commentedon Feb 18, 2021
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.
berkerpeksag commentedon Feb 18, 2021
New changeset 47feb1f by Erlend Egeberg Aasland in branch 'master':
bpo-43249: sqlite3_column_bytes() must follow sqlite_column_blob() (GH-24562)
47feb1f
berkerpeksag commentedon Feb 18, 2021
New changeset cc96231 by Erlend Egeberg Aasland in branch 'master':
bpo-43249: Improve scoping in _pysqlite_fetch_one_row() (GH-24565)
cc96231