bpo-38185: Fixed case-insensitive string comparison in sqlite3.Row indexing.#16190
Conversation
berkerpeksag
left a comment
There was a problem hiding this comment.
Nice find! The PR looks good to me. I probably wouldn't bother fixing it in 3.7, but it's ultimately up to you :)
| if (!PyUnicode_Check(left) || !PyUnicode_Check(right)) { | ||
| return 0; | ||
| } | ||
| if (!PyUnicode_IS_ASCII(left) || !PyUnicode_IS_ASCII(right)) { |
There was a problem hiding this comment.
Hmm, looks like sqlite supports non-ascii characters as identifiers: https://github.com/mackyle/sqlite/blob/a419afd73a544e30df878db55f7faa17790c01bd/ext/misc/normalize.c#L174-L177
And as shown in your example on bpo, should we maybe just simplify this code and do a casefold comparison?
There was a problem hiding this comment.
My initial version used casefold. But then I made some experiments and found that Sqlite itself ignores case only for ASCII letters. It was just my guess, and thank you for confirming it with the reference to sources.
There was a problem hiding this comment.
Aah okay, I thought the case-insensitivity for the Row object was for the convenience of the Python API, not trying to mirror any particular part of sqlite itself. Which part of sqlite ignores case?
There was a problem hiding this comment.
Which part of sqlite ignores case?
I guess any part which compare identifiers. For example, the following code passes:
create table test(a text)
select A from testand the following fails:
create table test(a text, A text)There was a problem hiding this comment.
Aah gotcha, thanks for taking the time to explain Serhiy.
|
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8. |
…dexing. (pythonGH-16190) (cherry picked from commit f669581) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
GH-16216 is a backport of this pull request to the 3.8 branch. |
…dexing. (pythonGH-16190) (cherry picked from commit f669581) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
GH-16217 is a backport of this pull request to the 3.7 branch. |
|
It could be fixed even in 2.7, but I don't bother. The bug is too tiny. |
https://bugs.python.org/issue38185