GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||
GeezerVison is showing github.com — bpo-44822: Don't truncate `str`s with embedded NULL chars returned by `sqlite3` UDF callbacks by erlend-aasland · Pull Request #27588 · python/cpython Conversation
| ||||||||||||||||||||||||
|
Ah, yes, we need a news entry now that we raise |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
[x] Please add a NEWS entry. It is now a bug fix, not a merely micro-optimization.
Would be nice to add also tests for strings with lengths INT_MAX and INT_MAX+1. I am not sure SQLite works with INT_MAX, it will likely raise an error. Use the bigmemtest decorator from test.support, because tests will consume at least 2GiB of memory (maybe more if the case with INT_MAX passes).
If you have problems with writing such tests (if they require much more than 4GiB of memory), no problem, I'll write them.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Nice, I was unaware of that support function. I've added tests with diff --git a/Lib/sqlite3/test/userfunctions.py b/Lib/sqlite3/test/userfunctions.py index b3da3c425b..6d72295491 100644 --- a/Lib/sqlite3/test/userfunctions.py +++ b/Lib/sqlite3/test/userfunctions.py @@ -31,0 +32,2 @@ +from test.support import bigmemtest, _2G, _4G + @@ -231,0 +234,9 @@ def test_func_return_text_with_null_char(self): + @bigmemtest(size=_4G, memuse=1, dry_run=False) + def test_func_bigmem_text(self): + cur = self.con.cursor() + self.con.create_function("bigmem2g", 0, lambda: "b" * _2G) + self.con.create_function("bigmem4g", 0, lambda: "b" * _4G) + res = cur.execute("select bigmem2g()").fetchone()[0] + self.assertEqual(len(res), _2G) + self.assertRaises(OverflowError, cur.execute, "select bigmem4g()") + Feel free to add the tests if you are able to run bigmem tests locally. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
👍
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| @@ -0,0 +1,3 @@ | |||
| :mod:`sqlite3` now raises :exc:`OverflowError` if an user-defined function | |||
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Actually sqlite3 overwrites OverflowError with other exception. But it does not matter, the most important part of this PR is that result strings with NUL are now correctly supported.
Something like (please correct my English):
| :mod:`sqlite3` now raises :exc:`OverflowError` if an user-defined function | |
| Fix support of user functions and aggregates returning string containing NUL in :mod:`sqlite3`. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Actually sqlite3 overwrites OverflowError with other exception.
Ah, yes, there should be some exception chaining in the UDF callbacks. I'll open an issue for it.
Anyway, how does the reworded NEWS entry look?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
9a54953 to
55963ca
Compare
|
I have made the requested changes; please review again. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
strs with embedded NULL chars returned by sqlite3 UDF callbacks
There was an error while loading. Please reload this page.
|
Thanks @erlend-aasland for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry, @erlend-aasland and @serhiy-storchaka, I could not cleanly backport this to |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
GH-27611 is a backport of this pull request to the 3.10 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
… `sqlite3` UDF callbacks (pythonGH-27588) (cherry picked from commit 8f010dc) Co-authored-by: Erlend Egeberg Aasland <[email protected]>
|
Thank you for your review and suggestions, Serhiy. Much appreciated! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
In order to backport this to 3.9, we also need to backport #25422. Either we first backport #25422 to 3.9 and then backport this to 3.9, or we don't backport this PR to 3.9. Let me know what you think, @serhiy-storchaka. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
👍 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
… `sqlite3` UDF callbacks (GH-27588) (cherry picked from commit 8f010dc) Co-authored-by: Erlend Egeberg Aasland <[email protected]>
|
Thanks @erlend-aasland for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry, @erlend-aasland and @serhiy-storchaka, I could not cleanly backport this to |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
GH-27639 is a backport of this pull request to the 3.9 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…ned by `sqlite3` UDF callbacks (pythonGH-27588). (cherry picked from commit 8f010dc) Co-authored-by: Erlend Egeberg Aasland <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.