8000 bpo-44108: normalise SQL quoted literals in sqlite3 test suite by erlend-aasland · Pull Request #26032 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content
10000

bpo-44108: normalise SQL quoted literals in sqlite3 test suite #26032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2021

Conversation

erlend-aasland
Copy link
Contributor
@erlend-aasland erlend-aasland commented May 11, 2021

This makes the sqlite3 test suite compatible with SQLite libraries compiled
with SQLITE_DQS=0 defined.

See also:

https://bugs.python.org/issue44108

@erlend-aasland
Copy link
Contributor Author
erlend-aasland commented May 11, 2021

@corona10 I think this can pass as skip news: The test now uses single iso. double quoted literals; the test is still the same; there is no user visible change.

May I add skip news, @corona10?

@erlend-aasland
Copy link
Contributor Author
erlend-aasland commented May 11, 2021

@corona10 Regarding backporting, I'd say backport only to 3.10. EDIT: possibly backport to 3.10.

@corona10
Copy link
Member

May I add skip news, @corona10?

@erlend-aasland Go a head :)

@erlend-aasland
Copy link
Contributor Author

@erlend-aasland Go a head :)

I've changed my mind! :) This should be mentioned with a NEWS item: after this PR, the sqlite3 test suite will now be compatible with SQLite libraries compiled with SQLITE_DQS=0. It's a tiny, tiny feature. So, I say: No backport labels, no skip news label. What do you think?

@corona10
Copy link
Member
corona10 commented May 13, 2021

the sqlite3 test suite will now be compatible with SQLite libraries compiled with SQLITE_DQS=0.

We often don't add NEWS with test change but if this is a special case please let me know.

@erlend-aasland
Copy link
Contributor Author

We often don't add NEWS with test change but if this is a special case please let me know.

Hm, ok. The average user is of course not expected to run the CPython test suite with a self-compiled SQLite library. I'm just thinking something like "The sqlite3 module now supports SQLite libraries compiled with SQLITE_DQS=0".

@erlend-aasland
Copy link
Contributor Author

the sqlite3 test suite will now be compatible with SQLite libraries compiled with SQLITE_DQS=0.

We often don't add NEWS with test change but if this is a special case please let me know.

@berkerpeksag what's your opinion on this?

@corona10
Copy link
Member
corona10 commented May 14, 2021

"The sqlite3 module now supports SQLite libraries compiled with SQLITE_DQS=0"

CPython already supports SQLITE_DQS=0 but only failed at the test suite, right?
(Please let me know if I wrong)
There looks like no modification from the CPython module side but only the test suite.

@erlend-aasland
Copy link
Contributor Author

CPython already supports SQLITE_DQS=0 but only failed at the test suite, right?

(Please let me know if I wrong)

There looks like no modification from the CPython module side but only the test suite.

Yeah, that's correct. I guess it's no NEWS then.

@corona10
Copy link
Member
corona10 commented May 14, 2021

@erlend-aasland
I'd like to make backport patch.
Which version was the oldest version (> 3.8) to be affected?

@erlend-aasland
Copy link
Contributor Author
erlend-aasland commented May 14, 2021

@erlend-aasland

I'd like to make backport patch.

Which version was the oldest version (> 3.8) to be affected?

I'll check when I'm back on my computer, but I'll bet that the test we are adjusting has been there since before 3.8. Note that Miss Islington might fail to automatically backport to 3.9 and 3.8 (we normalised the test case names in 3.10).

But I wouldn't backport to 3.8. At most 3.9.

@corona10 corona10 added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels May 14, 2021
@corona10
Copy link
Member

@erlend-aasland

Miss Islington might fail to automatically backport to 3.9

if failed please submit the manual backport patch.

But I wouldn't backport to 3.8.

3.8 is security only status.
https://devguide.python.org/#status-of-python-branches

@corona10 corona10 merged commit be7e467 into python:main May 14, 2021
@miss-islington
Copy link
Contributor

Thanks @erlend-aasland for the PR, and @corona10 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @erlend-aasland and @corona10, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker be7e467bcf5e419302d887904ef3e8fd310c68e7 3.9

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label May 14, 2021
@bedevere-bot
Copy link

GH-26125 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 14, 2021
(cherry picked from commit be7e467)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
@erlend-aasland erlend-aasland deleted the sqlite-dqs branch May 14, 2021 10:27
@erlend-aasland
Copy link
Contributor Author

if failed please submit the manual backport patch.

Of course :) I'll do it when I'm back on my computer.

3.8 is security only status.

Exactly :)

Thanks for helping me sorting out the NEWS & backport issues, Dong-hee!

corona10 pushed a commit that referenced this pull request May 14, 2021
…H-26125)

(cherry picked from commit be7e467)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request May 14, 2021
(cherry picked from commit be7e467)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
@bedevere-bot
Copy link

GH-26128 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label May 14, 2021
corona10 pushed a commit that referenced this pull request May 14, 2021
…26128)

(cherry picked from commit be7e467)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0