Repository navigation
[sqlite3] sqlite3_column_name() failures should raise MemoryError #87417
Description
| BPO | 43251 |
|---|---|
| Nosy | @berkerpeksag, @serhiy-storchaka, @erlend-aasland |
| PRs |
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 = None
closed_at = <Date 2021-02-28.17:01:29.187>
created_at = <Date 2021-02-18.11:22:34.286>
labels = ['type-feature', 'library', '3.10']
title = '[sqlite3] sqlite3_column_name() failures should raise MemoryError'
updated_at = <Date 2021-02-28.17:01:29.186>
user = 'https://github.com/erlend-aasland'bugs.python.org fields:
activity = <Date 2021-02-28.17:01:29.186>
actor = 'berker.peksag'
assignee = 'none'
closed = True
closed_date = <Date 2021-02-28.17:01:29.187>
closer = 'berker.peksag'
components = ['Library (Lib)']
creation = <Date 2021-02-18.11:22:34.286>
creator = 'erlendaasland'
dependencies = []
files = []
hgrepos = []
issue_num = 43251
keywords = ['patch']
message_count = 15.0
messages = ['387214', '387336', '387347', '387402', '387403', '387405', '387428', '387446', '387447', '387565', '387566', '387567', '387591', '387620', '387810']
nosy_count = 3.0
nosy_names = ['berker.peksag', 'serhiy.storchaka', 'erlendaasland']
pr_nums = ['24609']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue43251'
versions = ['Python 3.10']
Activity
erlend-aasland commentedon Feb 18, 2021
AFAICS, the docs for sqlite3_column_name() says that if it returns NULL, we are out of memory, thus we should call PyErr_NoMemory() and bail.
FYI, there are the calls to sqlite3_column_name() in Modules/_sqlite//cursor.c
Ref:
[-]sqlite3_column_name() failures should call PyErr_NoMemory()[/-][+]sqlite3_column_name() failures should raise MemoryError[/+][-]sqlite3_column_name() failures should call PyErr_NoMemory()[/-][+]sqlite3_column_name() failures should raise MemoryError[/+]serhiy-storchaka commentedon Feb 19, 2021
Well, it returns NULL in case of out of memory, but is it the only cause? Can NULL be returned for other reasons?
erlend-aasland commentedon Feb 19, 2021
According to the SQLite docs, no. Looking at the source code, we see that it also returns NULL if the second parameter (column index) is out of range, but we already check that. AFAICS, the only reason for a NULL in our case is OOM.
[-]sqlite3_column_name() failures should raise MemoryError[/-][+][sqlite3] sqlite3_column_name() failures should raise MemoryError[/+][-]sqlite3_column_name() failures should raise MemoryError[/-][+][sqlite3] sqlite3_column_name() failures should raise MemoryError[/+]serhiy-storchaka commentedon Feb 20, 2021
It also returns NULL if the column name was not set (is it even possible?).
erlend-aasland commentedon Feb 20, 2021
That's inside sqlite3_value_text() and friends, then? Let's investigate further before concluding.
serhiy-storchaka commentedon Feb 20, 2021
Investigate where column names are initialized and whether is it possible to get them NULLs. The array of column names is filled with NULLs when initialized and later it is filled with references to strings. I am wondering whether there is a gap between this which lefts some column names as NULLs.
erlend-aasland commentedon Feb 20, 2021
"SELECT 1" yields "1" as column name.
"SELECT <...> AS N" yields "N" as column name.
"SELECT NULL" yields "NULL" as column name.
We can't set the column name via the API, so that's pretty much it (implicit name or explicit through the AS keyword). As long as sqlite3_prepare_v2() returned SQLITE_OK, it seems that SQLite assures we've got a valid column name.
As far as I can sqlite3_column_name() returns NULL if:
We've got 1) and 2) covered. SQLite has a pretty good code coverage, so I'd say 4) is unlikely.
What do you think?
serhiy-storchaka commentedon Feb 21, 2021
Please go ahead!
erlend-aasland commentedon Feb 21, 2021
Will do. Thanks for pushing the investigation.
erlend-aasland commentedon Feb 23, 2021
After discussing the matter briefly on the SQLite forum, I'm no longer 100% certain about this. There seems to be uncertainty about which other conditions can produce NULL, although memory error seems to be the most probable. I consider closing both this issue and the PR and just leave the code as it is.
serhiy-storchaka commentedon Feb 23, 2021
Could you please give a link to the discussion?
erlend-aasland commentedon Feb 23, 2021
Sure! Here it is: https://sqlite.org/forum/forumpost/574c6bc66c
erlend-aasland commentedon Feb 23, 2021
Larry Brasfield's comment https://sqlite.org/forum/forumpost/6430fc589d?t=h aligns with https://bugs.python.org/issue43251#msg387428
I'll think twice before posting there again, though.
erlend-aasland commentedon Feb 24, 2021
I believe we can proceed with this as planned. Serhiy, do you have additional comments or change requests?
berkerpeksag commentedon Feb 28, 2021
New changeset 2183d06 by Erlend Egeberg Aasland in branch 'master':
bpo-43251: sqlite3_column_name() failures now raise MemoryError (GH-24609)
2183d06