[issue43181] Python macros don’t shield arguments
Erlend Egeberg Aasland added the comment: > If possible, I would prefer to only replace macros with static inline > functions if the changes avoids clear macro pitfalls. Yes, of course. Should I create a meta issue for this, or do you prefer raising one issue pr. fix? -- ___ Python tracker <https://bugs.python.org/issue43181> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15698] PEP 3121, 384 Refactoring applied to pyexpat module
Erlend Egeberg Aasland added the comment: Fixed by GH-2. -- nosy: +erlendaasland, vstinner ___ Python tracker <https://bugs.python.org/issue15698> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12777] Inconsistent use of VOLUME_NAME_* with GetFinalPathNameByHandle
Erlend Egeberg Aasland added the comment: This seems to have been fixed in 2018 in bpo-33016 by GH-6010. -- nosy: +erlendaasland ___ Python tracker <https://bugs.python.org/issue12777> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1103350] send/recv SEGMENT_SIZE should be used more in socketmodule
Erlend Egeberg Aasland added the comment: FYI, the SEGMENT_SIZE define was removed by af01f668173d4061893148b54a0f01b91c7716c2 (bpo-16136). -- nosy: +erlendaasland ___ Python tracker <https://bugs.python.org/issue1103350> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43181] Python macros don’t shield arguments
Erlend Egeberg Aasland added the comment: > As I wrote previously, I dislike macros. If someone is changed, I would > prefer to convert the function into a static inline function which doesn't > have macros pitfalls. Should we create a meta issue for this? Most macros are trivial, and can safely be converted, given that they're not used as l-values. Converting one header file at the time would make it easy to review, and it would be possible to make good progress in a few months time. -- ___ Python tracker <https://bugs.python.org/issue43181> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue42686] include built-in Math functions in SQLite to 3.35.0 of march 2021
Erlend Egeberg Aasland added the comment: See bpo-43492 for upgrading the macOS & Windows installers to SQLite 3.35.0. -- ___ Python tracker <https://bugs.python.org/issue42686> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43492] Upgrade to SQLite 3.35.0 in macOS and Windows
New submission from Erlend Egeberg Aasland : SQLite 3.35.0 was released a couple of days ago: https://www.sqlite.org/releaselog/3_35_0.html Suggesting to hold off for a week or two, to see if a bug-fix release happens. -- components: Windows, macOS messages: 388685 nosy: erlendaasland, ned.deily, paul.moore, ronaldoussoren, steve.dower, tim.golden, zach.ware priority: normal severity: normal status: open title: Upgrade to SQLite 3.35.0 in macOS and Windows type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43492> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43267] [sqlite3] Redundant type checks in pysqlite_statement_bind_parameter()
Erlend Egeberg Aasland added the comment: $ python -m pyperf compare_to -G main.json patched.json Faster (1): - bind: 63.6 us +- 1.2 us -> 61.8 us +- 0.9 us: 1.03x faster $ git diff --shortstat master 1 file changed, 41 insertions(+), 74 deletions(-) -- keywords: +patch Added file: https://bugs.python.org/file49870/patch.diff ___ Python tracker <https://bugs.python.org/issue43267> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43454] [sqlite3] Add support for R*Tree callbacks
Erlend Egeberg Aasland added the comment: Unfortunately, there's no way to detect R*Tree support in sqlite3.h. We could run a script that dumps the compile options, and generate a config.h file from that: $ for L in $(sqlite3 ":memory:" "pragma compile_options"); do echo #define SQLITE_$L >> config.h; done $ cat config.h #define SQLITE_COMPILER=clang-12.0.0 #define SQLITE_ENABLE_DBSTAT_VTAB #define SQLITE_ENABLE_FTS4 #define SQLITE_ENABLE_FTS5 #define SQLITE_ENABLE_GEOPOLY #define SQLITE_ENABLE_JSON1 #define SQLITE_ENABLE_RTREE #define SQLITE_ENABLE_STMTVTAB #define SQLITE_THREADSAFE=1 For Windows and macOS builds, we've already got the SQLite library compile options, so we can just reuse them in CFLAGS when we're building the sqlite3 module. -- ___ Python tracker <https://bugs.python.org/issue43454> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43454] [sqlite3] Add support for R*Tree callbacks
Erlend Egeberg Aasland added the comment:
Test run output (see attached test file):
$ ./python.exe test_rtree.py
ARGS: ((-80.77490234375, -80.77469635009766, 35.377593994140625,
35.377803802490234), (45.3, 22.9, 5.0))
KWARGS: {'num_queued': [0, 1], 'context': None, 'level': 0, 'max_level': 1,
'rowid': 1, 'parent_score': 0.0, 'parent_visibility': 1}
ARGS: ((-81.0, -79.584741211, 35.0, 36.2076293945), (45.3, 22.9, 5.0))
KWARGS: {'num_queued': [1, 1], 'context': ['stuff'], 'level': 0, 'max_level':
1, 'rowid': 2, 'parent_score': 0.0, 'parent_visibility': 1}
--
Added file: https://bugs.python.org/file49867/test_rtree.py
___
Python tracker
<https://bugs.python.org/issue43454>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43454] [sqlite3] Add support for R*Tree callbacks
Erlend Egeberg Aasland added the comment: FYI, PoC patch attached. Lacks tests and some #ifdefs. Let me know if I should create a PR out of it. -- keywords: +patch Added file: https://bugs.python.org/file49866/patch.diff ___ Python tracker <https://bugs.python.org/issue43454> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43440] Enable rtree support in SQLite
Erlend Egeberg Aasland added the comment: I've opened bpo-43454 for R*Tree callbacks. -- ___ Python tracker <https://bugs.python.org/issue43440> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43454] [sqlite3] Add support for R*Tree callbacks
New submission from Erlend Egeberg Aasland : Ref. bpo-43440 Now that both Windows and macOS builds compile SQLite with R*Tree support, we should consider adding support for R*Tree callbacks. SQLite has two API's: - sqlite3_rtree_query_callback() for SQLite 3.8.5 and newer. - sqlite3_rtree_geometry_callback() for SQLite pre 3.8.5. I suggest using the new API only, because it is more flexible, and it is also the one recommended by SQLite. See https://sqlite.org/rtree.html Python API: sqlite3.Connection.create_rtree_query_function() Too long function name? As for the callback spec, I'm not sure what's the most pythonic approach? callback(coords, *params, **query_info): coords # array of coordinates of node or entry to check *params # parameters passed to the SQL function **query_info # the rest of the relevant sqlite3_rtree_query_info members return (visibility, score) -- components: Library (Lib) messages: 388391 nosy: berker.peksag, erlendaasland priority: normal severity: normal status: open title: [sqlite3] Add support for R*Tree callbacks type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43454> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43440] Enable rtree support in SQLite
Erlend Egeberg Aasland added the comment: Anytime :) I'll create an issue for rtree callbacks. -- ___ Python tracker <https://bugs.python.org/issue43440> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43444] [sqlite3] Move MODULE_NAME def from setup.py to module.h
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23569 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24801 ___ Python tracker <https://bugs.python.org/issue43444> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43440] Enable rtree support in SQLite
Erlend Egeberg Aasland added the comment: > does compile options set in setup.py propagate to the Windows build, or do we > need to set it both places? Er, forget it. setup.py is of course only used to build the sqlite3 module, not sqlite3. -- ___ Python tracker <https://bugs.python.org/issue43440> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43444] [sqlite3] Move MODULE_NAME def from setup.py to module.h
New submission from Erlend Egeberg Aasland : Berker, can we please move the MODULE_NAME define from setup.py to Modules/_sqlite/module.h? I'm tired of all the undeclared identifier warnings. No other module defines their MODULE_NAME in setup.py. -- components: Library (Lib) messages: 388344 nosy: berker.peksag, erlendaasland priority: normal severity: normal status: open title: [sqlite3] Move MODULE_NAME def from setup.py to module.h type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43444> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43440] Enable rtree support in SQLite
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23565 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24797 ___ Python tracker <https://bugs.python.org/issue43440> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43440] Enable rtree support in SQLite
Erlend Egeberg Aasland added the comment: We should also consider adding support for R*Tree query callbacks using sqlite3_rtree_query_callback() for SQLite >= 3.8.5, and sqlite3_rtree_geometry_callback() for older versions. -- ___ Python tracker <https://bugs.python.org/issue43440> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43440] Enable rtree support in SQLite
Erlend Egeberg Aasland added the comment: > does compile options set in setup.py propagate to the Windows build, or do we > need to set it both places? Seems like it doesn't; correct me if I'm wrong. PR coming up. -- ___ Python tracker <https://bugs.python.org/issue43440> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43440] Enable rtree support in SQLite
Erlend Egeberg Aasland added the comment: Actually, the macOS build already builds with R*Tree support enabled, but it is missing from PCbuild/sqlite3.vcxproj. I'm not very familiar with the Windows build; does compile options set in setup.py propagate to the Windows build, or do we need to set it both places? -- ___ Python tracker <https://bugs.python.org/issue43440> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43440] Enable rtree support in SQLite
Erlend Egeberg Aasland added the comment: I can put up a PR for it. I don’t see any reason not to enable it. -- ___ Python tracker <https://bugs.python.org/issue43440> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43440] Enable rtree support in SQLite
Erlend Egeberg Aasland added the comment: Unless I’m mistaken, that’s enabled simply by compiling with SQLITE_ENABLE_RTREE defined. -- ___ Python tracker <https://bugs.python.org/issue43440> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43434] sqlite3.Connection(...) bypasses 'sqlite3.connect' audit hooks
Change by Erlend Egeberg Aasland : -- keywords: +patch Added file: https://bugs.python.org/file49858/patch.diff ___ Python tracker <https://bugs.python.org/issue43434> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43434] sqlite3.Connection(...) bypasses 'sqlite3.connect' audit hooks
New submission from Erlend Egeberg Aasland :
The module level connect method is guarded by PySys_Audit(), but
sqlite3.Connection.__init__() is not. It is possible to bypass the module level
connect() method simply by creating a new sqlite3.Connection object directly.
Easily fixed by either moving the PySys_Audit() check to
pysqlite_connection_init(), or by adding an extra check in
pysqlite_connection_init().
>>> import sqlite3, sys
>>> def hook(s, e):
... if s == 'sqlite3.connect':
... raise PermissionError
...
>>> sys.addaudithook(hook)
>>> sqlite3.connect(':memory:')
Traceback (most recent call last):
File "", line 1, in
File "", line 3, in hook
PermissionError
>>> sqlite3.Connection(':memory:')
--
components: Library (Lib)
files: audit.py
messages: 388264
nosy: berker.peksag, erlendaasland, steve.dower
priority: normal
severity: normal
status: open
title: sqlite3.Connection(...) bypasses 'sqlite3.connect' audit hooks
type: security
versions: Python 3.10, Python 3.8, Python 3.9
Added file: https://bugs.python.org/file49857/audit.py
___
Python tracker
<https://bugs.python.org/issue43434>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43396] Use more descriptive variable names in sqlite3 docs
Change by Erlend Egeberg Aasland : -- pull_requests: +23516 stage: needs patch -> patch review pull_request: https://github.com/python/cpython/pull/24746 ___ Python tracker <https://bugs.python.org/issue43396> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43396] Non-existent method sqlite3.Connection.fetchone() used in docs
Erlend Egeberg Aasland added the comment: Attached patch consists of two commits: one to rename "conn" to "con", and one to rename "c" to "cur". Are you ok with that, Berker? -- Added file: https://bugs.python.org/file49851/patch.diff ___ Python tracker <https://bugs.python.org/issue43396> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43396] Non-existent method sqlite3.Connection.fetchone() used in docs
Erlend Egeberg Aasland added the comment: 'con' and 'cur' seems to be used in a majority of the examples. I suggest normalising to always using these two in code examples. -- ___ Python tracker <https://bugs.python.org/issue43396> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43396] Non-existent method sqlite3.Connection.fetchone() used in docs
Erlend Egeberg Aasland added the comment: Right, got me confused as well! I guess a clarification would be an improvement :) -- ___ Python tracker <https://bugs.python.org/issue43396> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43398] [sqlite3] sqlite3.connect() segfaults if given a faulty Connection factory
New submission from Erlend Egeberg Aasland :
If the connection factory __init__ method fails, we hit a seg. fault when
pysqlite_do_all_statements() is called to clean up the defect connection:
PyList_Size received a NULL pointer.
Suggested fix:
Split pysqlite_do_all_statements() in two: one function for resetting cursors,
and one for resetting/finalising statements. In each function, check if the
respective lists are NULL pointers before iterating. See attached proposed
patch.
Test:
def test_invalid_connection_factory(self):
class DefectFactory(sqlite.Connection):
def __init__(self, *args, **kwargs):
return None
self.con = sqlite.connect(":memory:", factory=DefectFactory)
--
components: Library (Lib)
files: patch.diff
keywords: patch
messages: 388082
nosy: berker.peksag, erlendaasland, serhiy.storchaka
priority: normal
severity: normal
status: open
title: [sqlite3] sqlite3.connect() segfaults if given a faulty Connection
factory
type: crash
versions: Python 3.10
Added file: https://bugs.python.org/file49850/patch.diff
___
Python tracker
<https://bugs.python.org/issue43398>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43396] Non-existent method sqlite3.Connection.fetchone() used in docs
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23515 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24745 ___ Python tracker <https://bugs.python.org/issue43396> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43396] Non-existent method sqlite3.Connection.fetchone() used in docs
Erlend Egeberg Aasland added the comment: Correct, fetchone() is a cursor method. Thanks for the report! -- nosy: +berker.peksag, erlendaasland ___ Python tracker <https://bugs.python.org/issue43396> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43369] [sqlite3] Handle out-of-memory errors in sqlite3_column_*()
Erlend Egeberg Aasland added the comment: Quoting from the SQLite docs: "As long as the input parameters are correct, these routines will only fail if an out-of-memory error occurs during a format conversion" -- ___ Python tracker <https://bugs.python.org/issue43369> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43369] [sqlite3] Handle out-of-memory errors in sqlite3_column_*()
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23496 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24723 ___ Python tracker <https://bugs.python.org/issue43369> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43060] Convert _decimal C API from pointer array to struct
Erlend Egeberg Aasland added the comment: > But converting the decimal c api may breaks the compatibility, because some > macros like `PyDec_TypeCheck_INDEX` have been exposed in headers. True. Is there many external users of this API? I could not find any relevant examples using searchcode.com. -- ___ Python tracker <https://bugs.python.org/issue43060> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43350] [sqlite3] Active statements are reset twice in _pysqlite_query_execute()
Erlend Egeberg Aasland added the comment: Hm, I guess we could use sqlite3_stmt_busy() instead of the in_use flag. -- ___ Python tracker <https://bugs.python.org/issue43350> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43350] [sqlite3] Active statements are reset twice in _pysqlite_query_execute()
Erlend Egeberg Aasland added the comment: > There are three calls of pysqlite_statement_reset() in > _pysqlite_query_execute() Yes, but only two before the cache is queried. > So additional call of pysqlite_statement_reset() does not harm. That's true, but removing redundant code makes it easier to read and maintain, IMO. > On other hand removing it can introduce race condition. How? > Maybe the code could be rewritten in more explicit way and call > pysqlite_statement_reset() only when it is necessary Most of _pysqlite_query_execute() could be rewritten in order to make the code clearer. An alternative is to clean it up part by part. -- ___ Python tracker <https://bugs.python.org/issue43350> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43350] [sqlite3] Active statements are reset twice in _pysqlite_query_execute()
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23466 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24681 ___ Python tracker <https://bugs.python.org/issue43350> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43350] [sqlite3] Active statements are reset twice in _pysqlite_query_execute()
New submission from Erlend Egeberg Aasland : In _pysqlite_query_execute(), if the cursor already has a statement object, it is reset twice before the cache is queried. -- components: Library (Lib) messages: 387850 nosy: berker.peksag, erlendaasland, serhiy.storchaka priority: normal severity: normal status: open title: [sqlite3] Active statements are reset twice in _pysqlite_query_execute() type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43350> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43349] [doc] incorrect tuning(7) manpage link
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23465 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24680 ___ Python tracker <https://bugs.python.org/issue43349> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43349] [doc] incorrect tuning(7) manpage link
Erlend Egeberg Aasland added the comment: > FreeBSD only (?) system call [...] Correction, it's not a sys call. It belongs to the misc manpage section. -- ___ Python tracker <https://bugs.python.org/issue43349> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43349] [doc] incorrect tuning(7) manpage link
New submission from Erlend Egeberg Aasland : In https://docs.python.org/3.10/library/resource.html#resource.RLIMIT_SWAP, tuning(7) points to https://manpages.debian.org/tuning(7), however this is a FreeBSD only (?) system call, so the link is incorrect. I suggest linking to either: - https://docs.freebsd.org/en/books/handbook/config/ - https://www.freebsd.org/cgi/man.cgi?query=tuning=7=html -- assignee: docs@python components: Documentation messages: 387845 nosy: docs@python, erlendaasland priority: normal severity: normal status: open title: [doc] incorrect tuning(7) manpage link versions: Python 3.10, Python 3.8, Python 3.9 ___ Python tracker <https://bugs.python.org/issue43349> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43296] [sqlite3] Fix sqlite3_value_blob() usage
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23459 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24674 ___ Python tracker <https://bugs.python.org/issue43296> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43296] [sqlite3] Fix sqlite3_value_blob() usage
Erlend Egeberg Aasland added the comment: > If sqlite3_value_blob() returns NULL, we should check if sqlite3_errcode() > equals SQLITE_NOMEM and raise MemoryError if it does. This also applies to sqlite3_value_text(). It also applies to sqlite3_value_bytes() if a conversion takes place. We don't need to care about that as long as we call sqlite3_value_bytes() after conversion. Also, PyTuple_SetItem() errors are not checked. I would also suggest using PyUnicode_FromStringAndSize() iso. PyUnicode_FromString() in case SQLITE_TEXT, to avoid the unneeded strlen(). After sqlite3_value_text() is called, we can just use sqlite3_value_bytes(), since the length is precomputed as a result of the conversion. Berker, would you allow a PR to improve all these issues in _pysqlite_build_py_params(), or would you prefer them split up in multiple PR's? -- ___ Python tracker <https://bugs.python.org/issue43296> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43296] [sqlite3] Fix sqlite3_value_blob() usage
Change by Erlend Egeberg Aasland : -- title: [sqlite3] sqlite3_value_bytes() should be called after sqlite3_value_blob() -> [sqlite3] Fix sqlite3_value_blob() usage ___ Python tracker <https://bugs.python.org/issue43296> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43294] [sqlite3] unused variables in Connection begin, commit, and rollback
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23443 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24658 ___ Python tracker <https://bugs.python.org/issue43294> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment: Addendum to msg387641: > The latter only leaves a valid Cursor->statement->st pointer (sqlite3_stmt > pointer) if the Statement object was successfully created, and the > sqlite3_stmt successfully prepared. sqlite3_prepare_v2() actually returns SQLITE_OK but sets the statement pointer to NULL, if given an empty string or a comment. Only the sqlite3_prepare_v2() return code is checked in order to determine if pysqlite_statement_create() was successful or not. -- ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment: msg387668 was a little bit hasty. I'll try again: Dong-hee Na: > Hmm by the way the current implementation returns SQLITE_OK if the statement > is NULL, but it looks like return SQLITE_MISUSE if we apply this patch. > Does it not cause any behavior regression? if so we should add news also. No, behaviour still stays the same; no regressions introduced. The bug is triggered by executing an empty statement. This will pass an empty statement to pysqlite_step() in line 519 of Modules/_sqlite/cursor.c. Earlier, this returned SQLITE_OK. sqlite3_column_count(NULL) returns 0, so we slide through the rest of the loop without much happening. Now, pysqlite_step() returns SQLITE_MISUSE, which only results in the statement being reset in line 529, and the error cleared in line 530. Then we bail out of the loop. So, the current comment is correct, the SQLite changelog was correct, the workaround and old comment was wrong. I'm done :) -- ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment:
> int rc = sqlite3_reset(NULL);
>printf("reset with NULL: %d %s\n", rc, sqlite3_errstr(rc));
Sorry, wrong test:
int rc = sqlite3_step(NULL);
printf("step with NULL: %d %s\n", rc, sqlite3_errstr(rc));
$ ./a.out
step with NULL: 21 bad parameter or other API misuse
--
___
Python tracker
<https://bugs.python.org/issue43290>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment:
Ah, at last I found the source of the confusion: The SQLite changelog.
Quoting from msg387619 and https://sqlite.org/changes.html:
> From the SQLite 3.5.3 changelog:
> - sqlite3_step() returns SQLITE_MISUSE instead of crashing when called with a
> NULL parameter.
I assumed this was correct without even trying it. This short snippet shows
something else:
int rc = sqlite3_reset(NULL);
printf("reset with NULL: %d %s\n", rc, sqlite3_errstr(rc));
$ ./a.out
reset with NULL: 0 not an error
Gerhard's comment was right and the workaround was right. I'll adjust the
comment.
Dong-hee Na:
> Hmm by the way the current implementation returns SQLITE_OK if the statement
> is NULL, but it looks like return SQLITE_MISUSE if we apply this patch.
> Does it not cause any behavior regression? if so we should add news also.
Behaviour stays the same; no regressions introduced. I learned a lot about the
sqlite3 module, and I relearned I should not trust changelogs/documentation
without trying stuff myself first.
I'll adjust the erroneous comment and re-request a review, Dong-hee.
--
___
Python tracker
<https://bugs.python.org/issue43290>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment: Introduced by Christian Heimes 2007-12-14 in commit 380532117c2547bb0dedf6f85efa66d18a9abb88, which is a merge from SVN (?) checkin r59471 by Gerhard Häring (https://mail.python.org/pipermail/python-checkins/2007-December/063968.html) This corresponds to https://github.com/ghaering/pysqlite/commit/61b3cd9381750bdd96f8f8fc2c1b19f1dc4acef7, with a simple test added two commits later, https://github.com/ghaering/pysqlite/commit/7b0faed4ababbf1053caa2f5b2efc15929f66c4f That test was added to CPython in 2008-03-29 by Gerhard in commit e7ea7451a84636655927da4b9731d2eb37d1cf34, and it's still here. -- ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment: > Look at issue in which the workaround was added. Does it contain some > examples? I'll check. Thanks. -- ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment:
> I wonder if it is possible at all to reach this branch. If it is not, then
> I'm pretty sure Cursor.in_use is redundant
Typo: should be Statement.in_use
Corner error cases may cause the _pysqlite_query_execute() loop to exit without
pysqlite_statement_reset() being called, thus leaving Statement.in_use == 1,
but when _pysqlite_query_execute() is called again and if there's an active
statement, pysqlite_statement_reset() will reset in_use to zero, before we ever
reach the if (self->statement->in_use) { ... } statement.
I can open a separate issue for considering removing Statement.in_use.
> (I assume only valid Statement objects are added to the cache.)
AFAICS, this is true.
We can wait for Berker and/or Serhiy to confirm/correct my assumptions and
findings.
--
___
Python tracker
<https://bugs.python.org/issue43290>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment: > _pysqlite_query_execute() has one coverage gap: lines 478-488 (if > (self->statement->in_use)) are never executed. I wonder if it is possible at all to reach this branch. If it is not, then I'm pretty sure Cursor.in_use is redundant, and all code relating to it can be removed. -- ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment: Regarding test coverage: _pysqlite_query_execute() has one coverage gap: lines 478-488 (if (self->statement->in_use)) are never executed. Except from that, the only missing are the hard-to-trigger goto error's (for example PyList_New and PyList_Append failures). I'm not sure if it's possible to trigger this, but I cannot see that it is relevant for our case here. Coverage for the other users of pysqlite_step is as complete as it can be; only the standard corner error cases are missing. -- ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment:
pysqlite_cursor_iternext() has four users:
- sqlite3.Cursor.fetchone()
- sqlite3.Cursor.fetchall()
- sqlite3.Cursor.fetchmany()
- sqlite3.Cursor.__next__()
All of these methods pass self to pysqlite_cursor_iternext().
pysqlite_cursor_iternext() starts by checking the state of the cursor (is it
initialised, it the database open, etc.). If there is a Statement object,
pysqlite_step() is called with the sqlite3_stmt pointer from the Statement
object (Cursor->statement->st).
The statement pointer of the Cursor object is set in _pysqlite_query_execute()
– the last pysqlite_step() user – either from the LRU cache (line 470), or by
creating a new Statement object (line 479). The latter only leaves a valid
Cursor->statement->st pointer (sqlite3_stmt pointer) if the Statement object
was successfully created, and the sqlite3_stmt successfully prepared. (I assume
only valid Statement objects are added to the cache.) Before the main loop of
_pysqlite_query_execute() starts, the statement is reset. In the loop, the next
parameter set is fetched, the statement is (re)bound, and step is called. If
Cursor.execute() called _pysqlite_query_execute(), the parameter list is
initialised to a single-item list, and the loop is only run once. From what I
can read, this function is also safe. (But it is very messy; for instance, if
there's an active Statement, it is reset twice before the loop starts.)
I tried forcing an error by using an uninitialised cursor:
>>> cx = sqlite3.connect(":memory:")
>>> cu = sqlite3.Cursor.__new__(sqlite3.Cursor)
>>> sqlite3.Cursor.fetchone(cu)
Traceback (most recent call last):
File "", line 1, in
sqlite3.ProgrammingError: Base Cursor.__init__ not called.
>>> next(cu)
Traceback (most recent call last):
File "", line 1, in
sqlite3.ProgrammingError: Base Cursor.__init__ not called.
Ditto for fetchmany() and fetchall(). This is consistent with current behaviour.
Calling fetch*() without first executing a statement:
>>> cu = cx.cursor()
>>> cu.fetchone()
>>> cu.fetchmany()
[]
>>> cu.fetchall()
[]
>>> next(cu)
Traceback (most recent call last):
File "", line 1, in
StopIteration
This is consistent with current behaviour.
I might have missed something, but from what I can see, there are no paths that
lead to pysqlite_step() being called with a NULL pointer.
Berker, Serhiy, please correct me if I'm wrong.
--
___
Python tracker
<https://bugs.python.org/issue43290>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment: There are six users of pysqlite_step(): $ grep -nrE "\" Modules/_sqlite Modules/_sqlite/util.c:27:int pysqlite_step(sqlite3_stmt* statement, pysqlite_Connection* connection) Modules/_sqlite/connection.c:393:rc = pysqlite_step(statement, self); Modules/_sqlite/connection.c:442:rc = pysqlite_step(statement, self); Modules/_sqlite/connection.c:493:rc = pysqlite_step(statement, self); Modules/_sqlite/util.h:32:int pysqlite_step(sqlite3_stmt* statement, pysqlite_Connection* connection); Modules/_sqlite/cursor.c:519:rc = pysqlite_step(self->statement->st, self->connection); Modules/_sqlite/cursor.c:715:rc = pysqlite_step(statement, self->connection); Modules/_sqlite/cursor.c:787:rc = pysqlite_step(self->statement->st, self->connection); The three users in Modules/_sqlite/connection.c — _pysqlite_connection_begin(), pysqlite_connection_commit_impl(), and pysqlite_connection_rollback_impl() – are all ok, following this pattern: 1) prepare the statement 2) verify that prepare was successful, bail if not 3) call step pysqlite_cursor_executescript() (line 715 in Modules/_sqlite/cursor.c) is also ok: 1) prepare the statement 2) verify that prepare was successful, bail if not 3) call step until there are no more rows I need a little bit more time to verify _pysqlite_query_execute() and pysqlite_cursor_iternext(). Note to self: pysqlite_cursor_executescript() calls sqlite3_finalize() three times. It would have been better to break out of the loop when rc != SQLITE_ROW, immediately call sqlite3_finalize() (error code is preserved if sqlite3_step() failed), and then check rc and PyErr_Occurred(). It will make the code easier to follow, IMO. -- ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment: I’ll check all uses and see if we’ve got everything covered by the test suite. This is one of the core functions of the sqlite3 module (all queries call step at least once, but often multiple times), so I’d expect coverage is pretty good. -- ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] [sqlite3] sqlite3_column_name() failures should raise MemoryError
Erlend Egeberg Aasland added the comment: I believe we can proceed with this as planned. Serhiy, do you have additional comments or change requests? -- ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23423 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24638 ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
Erlend Egeberg Aasland added the comment: >From the SQLite 3.5.3 changelog: - sqlite3_step() returns SQLITE_MISUSE instead of crashing when called with a NULL parameter. -- ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43314] [sqlite3] remove pre SQLite 3.7.7 support code
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23422 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24637 ___ Python tracker <https://bugs.python.org/issue43314> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43314] [sqlite3] remove pre SQLite 3.7.7 support code
New submission from Erlend Egeberg Aasland : I missed the SQLITE_OPEN_URI #ifdef in GH-24106. The "open URI" interface was added in 3.7.7. The dead code can safely be removed. $ grep -r SQLITE_OPEN_URI sqlite-3.7.6/ $ grep -r SQLITE_OPEN_URI sqlite-3.7.7/ [...] sqlite-3.7.7/sqlite3.h:#define SQLITE_OPEN_URI 0x0040 /* Ok for sqlite3_open_v2() */ -- components: Library (Lib) messages: 387616 nosy: berker.peksag, erlendaasland, serhiy.storchaka priority: normal severity: normal status: open title: [sqlite3] remove pre SQLite 3.7.7 support code type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43314> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] [sqlite3] sqlite3_column_name() failures should raise MemoryError
Erlend Egeberg Aasland added the comment: 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. -- ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] [sqlite3] sqlite3_column_name() failures should raise MemoryError
Erlend Egeberg Aasland added the comment: Sure! Here it is: https://sqlite.org/forum/forumpost/574c6bc66c -- ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] [sqlite3] sqlite3_column_name() failures should raise MemoryError
Erlend Egeberg Aasland added the comment: 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. -- ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue35056] Test leaks of memory not managed by Python allocator
Change by Erlend Egeberg Aasland : -- nosy: +erlendaasland ___ Python tracker <https://bugs.python.org/issue35056> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43296] [sqlite3] sqlite3_value_bytes() should be called after sqlite3_value_blob()
Erlend Egeberg Aasland added the comment: Related: If sqlite3_value_blob() returns NULL, we should check if sqlite3_errcode() equals SQLITE_NOMEM and raise MemoryError if it does. If not, we should initialise cur_py_value to None, because as the PyBytes_FromStringAndSize docs says: "If v is NULL, the contents of the bytes object are uninitialized." -- ___ Python tracker <https://bugs.python.org/issue43296> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24905] Allow incremental I/O to blobs in sqlite3
Change by Erlend Egeberg Aasland : -- nosy: +erlendaasland ___ Python tracker <https://bugs.python.org/issue24905> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43296] [sqlite3] sqlite3_value_bytes() should be called after sqlite3_value_blob()
New submission from Erlend Egeberg Aasland : The sqlite3_value_*() API is almost identical to the sqlite3_column_*() API. sqlite3_value_bytes() should be called after we've converted the value using sqlite3_value_blob(). See also bpo-43249. -- components: Library (Lib) messages: 387516 nosy: berker.peksag, erlendaasland, serhiy.storchaka priority: normal severity: normal status: open title: [sqlite3] sqlite3_value_bytes() should be called after sqlite3_value_blob() type: behavior versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43296> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43294] [sqlite3] unused variables in Connection begin, commit, and rollback
New submission from Erlend Egeberg Aasland : In Modules/_sqlite/connection.c: pysqlite_connection_commit_impl(), pysqlite_connection_rollback_impl(), and _pysqlite_connection_begin() all call sqlite3_prepare_v2() with the fourth parameter (pzTail) set. This (output) parameter is never used, we can safely remove the tail variables, and just call sqlite3_prepare_v2() with NULL as the fourth parameter. Also, there's a lot of code duplication here. A support function could help simplify this, but that's out of scope. -- components: Library (Lib) messages: 387510 nosy: berker.peksag, erlendaasland, serhiy.storchaka priority: normal severity: normal status: open title: [sqlite3] unused variables in Connection begin, commit, and rollback type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43294> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43290] [sqlite3] remove legacy code from pysqlite_step
New submission from Erlend Egeberg Aasland : pysqlite_step() contains a NULL check needed for SQLite 3.5 and earlier. This can be removed. -- components: Library (Lib) messages: 387476 nosy: berker.peksag, erlendaasland, serhiy.storchaka priority: normal severity: normal status: open title: [sqlite3] remove legacy code from pysqlite_step type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43290> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43267] [sqlite3] Redundant type checks in pysqlite_statement_bind_parameter()
Erlend Egeberg Aasland added the comment: > I am not sure that the difference is significant enough to justify the > optimization. I would guess it's negligible. I did some quick timeit tests, but the results were pretty much equal, so I guess I'd have to use a more precise benchmark method. > But the code is already written in such form, and I am not sure that it is > worth to spend our time to change it. Not even a 30% reduction in code size (for this function)? -- ___ Python tracker <https://bugs.python.org/issue43267> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] [sqlite3] sqlite3_column_name() failures should raise MemoryError
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23388 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24609 ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] [sqlite3] sqlite3_column_name() failures should raise MemoryError
Erlend Egeberg Aasland added the comment: Will do. Thanks for pushing the investigation. -- ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43269] [sqlite3] Clean up function scoping
Change by Erlend Egeberg Aasland : -- pull_requests: +23383 pull_request: https://github.com/python/cpython/pull/24605 ___ Python tracker <https://bugs.python.org/issue43269> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43269] [sqlite3] Clean up function scoping
Erlend Egeberg Aasland added the comment: Serhiy also mentioned that marking functions definitions with the extern keyword is not very common in the CPython source base. https://github.com/python/cpython/pull/24578#discussion_r579506678 I suggest cleaning up those as well, while we're at it. -- ___ Python tracker <https://bugs.python.org/issue43269> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] [sqlite3] sqlite3_column_name() failures should raise MemoryError
Erlend Egeberg Aasland added the comment: "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? -- ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43265] [sqlite3] Improve backup error handling
Erlend Egeberg Aasland added the comment: Regarding 1: After thinking about it, I see nothing wrong with raising ValueError error. No need to change current behaviour. -- ___ Python tracker <https://bugs.python.org/issue43265> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43265] [sqlite3] Improve backup error handling
Erlend Egeberg Aasland added the comment: 3.9, I presume? Yeah, I guess so. -- ___ Python tracker <https://bugs.python.org/issue43265> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] [sqlite3] sqlite3_column_name() failures should raise MemoryError
Erlend Egeberg Aasland added the comment: That's inside sqlite3_value_text() and friends, then? Let's investigate further before concluding. -- ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43265] [sqlite3] Improve backup error handling
Erlend Egeberg Aasland added the comment:
The unit test suite shows one case of improved "exception text". I'd say it's
an improvement.
$ ./python.exe # with GH-24586 applied
>>> import sqlite3
>>> c1 = sqlite3.connect(":memory:")
>>> c2 = sqlite3.connect(":memory:")
>>> c1.backup(c2, name="non-existing")
Traceback (most recent call last):
File "", line 1, in
sqlite3.OperationalError: unknown database non-existing
$ python3.10 # latest alpha from python.org
>>> import sqlite3
>>> c1 = sqlite3.connect(":memory:")
>>> c2 = sqlite3.connect(":memory:")
>>> c1.backup(c2, name="non-existing")
Traceback (most recent call last):
File "", line 1, in
sqlite3.OperationalError: SQL logic error
--
___
Python tracker
<https://bugs.python.org/issue43265>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43265] [sqlite3] Improve backup error handling
Change by Erlend Egeberg Aasland : -- title: Improve sqlite3 backup error handling -> [sqlite3] Improve backup error handling ___ Python tracker <https://bugs.python.org/issue43265> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] [sqlite3] sqlite3_column_name() failures should raise MemoryError
Change by Erlend Egeberg Aasland : -- title: sqlite3_column_name() failures should raise MemoryError -> [sqlite3] sqlite3_column_name() failures should raise MemoryError ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] sqlite3_column_name() failures should raise MemoryError
Erlend Egeberg Aasland added the comment: > 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. -- ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43267] [sqlite3] Redundant type checks in pysqlite_statement_bind_parameter()
Erlend Egeberg Aasland added the comment: > It is a kind of optimization. PyLong_Check is very fast (only one comparison, AFAICS), so there is no gain in first doing PyLong_CheckExact and then PyLong_Check. Ditto for unicode. PyFloat_Check is the most expensive check, but it comes before both PyUnicode_Check and PyObject_CheckBuffer (both a lot faster), so that's AFAICS the opposite of an optimisation. Correct me if I'm wrong. If we want to optimise it we should do PyLong_Check, PyUnicode_Check, PyObject_CheckBuffer, and then PyFloat_Check, no? > there is nothing wrong in it. True. I'll argue that my suggestion will improve readability and maintainability, which should be worth considering. -- ___ Python tracker <https://bugs.python.org/issue43267> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43265] Improve sqlite3 backup error handling
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23366 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24586 ___ Python tracker <https://bugs.python.org/issue43265> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43265] Improve sqlite3 backup error handling
Erlend Egeberg Aasland added the comment: > I'm not sure about 1) because if target == source it means a user error. > OperationalError is usually used for non-user errors. Yes, my bad. ProgrammingError would be better. target == source results in SQLITE_ERROR, BTW. I'll throw up a PR for 3., then. -- ___ Python tracker <https://bugs.python.org/issue43265> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43269] [sqlite3] Clean up function scoping
Erlend Egeberg Aasland added the comment: Ref. https://github.com/python/cpython/pull/24569#issuecomment-782014177, Berker. -- ___ Python tracker <https://bugs.python.org/issue43269> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43269] [sqlite3] Clean up function scoping
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23357 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24578 ___ Python tracker <https://bugs.python.org/issue43269> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43269] [sqlite3] Clean up function scoping
New submission from Erlend Egeberg Aasland : There's a lot of file scoped functions without the static storage-class specifier. All file local functions should have the static storage-class specifier. -- components: Library (Lib) messages: 387310 nosy: berker.peksag, erlendaasland priority: normal severity: normal status: open title: [sqlite3] Clean up function scoping type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43269> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43258] Prevent needless allocation of sqlite3 aggregate function context
Change by Erlend Egeberg Aasland : -- pull_requests: +23353 pull_request: https://github.com/python/cpython/pull/24574 ___ Python tracker <https://bugs.python.org/issue43258> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43267] [sqlite3] Redundant type checks in pysqlite_statement_bind_parameter()
New submission from Erlend Egeberg Aasland : The type checks at the start of pysqlite_statement_bind_parameter() are redundant: We first check for exact types (*_CheckExact()), and then we check again for exact or subtyped versions (*_Check()). (Adding to the redundantness: the result of this if..else "switch statement" is then reused as the expression in the subsequent switch statement.) Suggesting to remove the redundant checks and merge the two switch statements. -- components: Library (Lib) messages: 387302 nosy: berker.peksag, erlendaasland priority: normal severity: normal status: open title: [sqlite3] Redundant type checks in pysqlite_statement_bind_parameter() type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43267> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43251] sqlite3_column_name() failures should raise MemoryError
Change by Erlend Egeberg Aasland : -- title: sqlite3_column_name() failures should call PyErr_NoMemory() -> sqlite3_column_name() failures should raise MemoryError ___ Python tracker <https://bugs.python.org/issue43251> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43266] "String conversion and formatting" formatting messes up array subscripting
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23352 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24573 ___ Python tracker <https://bugs.python.org/issue43266> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43266] "String conversion and formatting" formatting messes up array subscripting
New submission from Erlend Egeberg Aasland : str[size-1] is rendered incorrectly in https://docs.python.org/3.10/c-api/conversion.html The original intent was probably to apply italics to the variables names only. This can be fixed by prefixing the first bracket with a backslash. However, that results in the italic text crashing with the bracket. I suggest to just use ``str[size-1]``. Also applies to str[rv] in the same section. -- assignee: docs@python components: Documentation messages: 387298 nosy: docs@python, erlendaasland priority: normal severity: normal status: open title: "String conversion and formatting" formatting messes up array subscripting versions: Python 3.10, Python 3.8, Python 3.9 ___ Python tracker <https://bugs.python.org/issue43266> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43265] Improve sqlite3 backup error handling
New submission from Erlend Egeberg Aasland : There are some issues with the error handling in pysqlite_connection_backup_impl(): 1. ValueError is returned if the target connection equals source connection. Should be OperationalError, IMHO. 2. The aforementioned check is already performed by sqlite3_backup_init(), so we should just let SQLite take care of it and let _pysqlite_seterror() set the error if sqlite3_backup_init() returns NULL. This will also take care of 1. 3. The following comment seems to be wrong; errors are set on the connection object, not on the backup handle: /* We cannot use _pysqlite_seterror() here because the backup APIs do not set the error status on the connection object, but rather on the backup handle. */ After sqlite3_backup_finish(), we can just check the return code, and call _pysqlite_seterror() on the connection and return NULL. The mentioned comment can be removed. Resolving these issues will save 18 lines of code, and make the backup function easier to maintain. Berker? -- components: Library (Lib) messages: 387297 nosy: berker.peksag, erlendaasland priority: normal severity: normal status: open title: Improve sqlite3 backup error handling type: enhancement versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43265> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43258] Prevent needless allocation of sqlite3 aggregate function context
Change by Erlend Egeberg Aasland : -- keywords: +patch pull_requests: +23348 stage: -> patch review pull_request: https://github.com/python/cpython/pull/24569 ___ Python tracker <https://bugs.python.org/issue43258> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43258] Prevent needless allocation of sqlite3 aggregate function context
New submission from Erlend Egeberg Aasland : If no rows match an aggregate query, _pysqlite_step_callback() is never called, and _pysqlite_final_callback() is called once. In order to prevent needless context allocation, we should pass 0 as the second argument to sqlite3_aggregate_context(). Quoting from https://sqlite.org/c3ref/aggregate_context.html: Within the xFinal callback, it is customary to set N=0 in calls to sqlite3_aggregate_context(C,N) so that no pointless memory allocations occur. -- components: Library (Lib) messages: 387272 nosy: berker.peksag, erlendaasland, serhiy.storchaka priority: normal severity: normal status: open title: Prevent needless allocation of sqlite3 aggregate function context versions: Python 3.10 ___ Python tracker <https://bugs.python.org/issue43258> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue43249] sqlite3_column_bytes() should be called after sqlite3_column_blob()
Change by Erlend Egeberg Aasland : -- pull_requests: +23346 pull_request: https://github.com/python/cpython/pull/24565 ___ Python tracker <https://bugs.python.org/issue43249> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
