classification
Title: [sqlite3] sqlite3_column_name() failures should raise MemoryError
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:22 by erlendaasland, last changed 2021-02-28 17:01 by berker.peksag. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 24609 merged erlendaasland, 2021-02-21 10:05
Messages (15)
msg387214 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-02-18 11:22
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:
- http://sqlite.org/c3ref/column_name.html
- http://sqlite.org/c3ref/free.html
msg387336 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-02-19 17:36
Well, it returns NULL in case of out of memory, but is it the only cause? Can NULL be returned for other reasons?
msg387347 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-02-19 20:12
> Well, it returns NULL in case of out of memory, but is it the only cause? Can NULL be returned for other reasons?

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.
msg387402 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-02-20 12:42
It also returns NULL if the column name was not set (is it even possible?).
msg387403 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-02-20 12:54
That's inside sqlite3_value_text() and friends, then? Let's investigate further before concluding.
msg387405 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-02-20 13:21
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.
msg387428 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-02-20 21:54
"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:
1) The first parameter (stmt) is NULL
2) The second parameter is out of bounds
3) Memory error
4) No column name is set (SQLite bug)

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?
msg387446 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-02-21 09:49
Please go ahead!
msg387447 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-02-21 10:03
Will do. Thanks for pushing the investigation.
msg387565 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-02-23 12:51
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.
msg387566 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-02-23 12:58
Could you please give a link to the discussion?
msg387567 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-02-23 13:04
Sure! Here it is: https://sqlite.org/forum/forumpost/574c6bc66c
msg387591 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-02-23 19:27
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.
msg387620 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-02-24 11:30
I believe we can proceed with this as planned. Serhiy, do you have additional comments or change requests?
msg387810 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2021-02-28 17:01
New changeset 2183d06bc8a481098d62a4ebed8d6982b3d1602a by Erlend Egeberg Aasland in branch 'master':
bpo-43251: sqlite3_column_name() failures now raise MemoryError (GH-24609)
https://github.com/python/cpython/commit/2183d06bc8a481098d62a4ebed8d6982b3d1602a
History
Date User Action Args
2021-02-28 17:01:29berker.peksagsetstatus: open -> closed
type: enhancement
resolution: fixed
stage: patch review -> resolved
2021-02-28 17:01:15berker.peksagsetmessages: + msg387810
2021-02-24 11:30:08erlendaaslandsetmessages: + msg387620
2021-02-23 19:27:05erlendaaslandsetmessages: + msg387591
2021-02-23 13:04:02erlendaaslandsetmessages: + msg387567
2021-02-23 12:58:11serhiy.storchakasetmessages: + msg387566
2021-02-23 12:51:38erlendaaslandsetmessages: + msg387565
2021-02-21 10:05:07erlendaaslandsetkeywords: + patch
stage: patch review
pull_requests: + pull_request23388
2021-02-21 10:03:13erlendaaslandsetmessages: + msg387447
2021-02-21 09:49:42serhiy.storchakasetmessages: + msg387446
2021-02-20 21:54:08erlendaaslandsetmessages: + msg387428
2021-02-20 13:21:26serhiy.storchakasetmessages: + msg387405
2021-02-20 12:54:29erlendaaslandsetmessages: + msg387403
2021-02-20 12:42:33serhiy.storchakasetmessages: + msg387402
2021-02-19 20:13:22erlendaaslandsettitle: sqlite3_column_name() failures should raise MemoryError -> [sqlite3] sqlite3_column_name() failures should raise MemoryError
2021-02-19 20:12:00erlendaaslandsetmessages: + msg387347
2021-02-19 17:36:41serhiy.storchakasetmessages: + msg387336
2021-02-19 10:24:53erlendaaslandsettitle: sqlite3_column_name() failures should call PyErr_NoMemory() -> sqlite3_column_name() failures should raise MemoryError
2021-02-18 11:22:34erlendaaslandcreate