Skip to content
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

sqlite3 test CheckTraceCallbackContent fails for sqlite v3.7.3 through 3.7.14.1 #84987

Closed
erlend-aasland opened this issue May 28, 2020 · 15 comments
Closed
Labels
3.8 3.9 3.10 stdlib type-feature

Comments

@erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented May 28, 2020

BPO 40810
Nosy @gvanrossum, @tiran, @berkerpeksag, @csabella, @corona10, @miss-islington, @brandtbucher, @erlend-aasland
PRs
  • #20330
  • #20530
  • #24104
  • #24105
  • #24106
  • #24110
  • #24111
  • #24112
  • #14491
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-01-06.00:04:50.476>
    created_at = <Date 2020-05-28.17:47:38.365>
    labels = ['3.8', 'type-feature', 'library', '3.9', '3.10']
    title = 'sqlite3 test CheckTraceCallbackContent fails for sqlite v3.7.3 through 3.7.14.1'
    updated_at = <Date 2021-01-06.00:05:08.010>
    user = 'https://github.com/erlend-aasland'

    bugs.python.org fields:

    activity = <Date 2021-01-06.00:05:08.010>
    actor = 'berker.peksag'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-01-06.00:04:50.476>
    closer = 'berker.peksag'
    components = ['Library (Lib)']
    creation = <Date 2020-05-28.17:47:38.365>
    creator = 'erlendaasland'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 40810
    keywords = ['patch']
    message_count = 15.0
    messages = ['370256', '370263', '370264', '370266', '370271', '370278', '370304', '384355', '384356', '384357', '384358', '384359', '384369', '384445', '384446']
    nosy_count = 8.0
    nosy_names = ['gvanrossum', 'christian.heimes', 'berker.peksag', 'cheryl.sabella', 'corona10', 'miss-islington', 'brandtbucher', 'erlendaasland']
    pr_nums = ['20330', '20530', '24104', '24105', '24106', '24110', '24111', '24112', '14491']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue40810'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @erlend-aasland
    Copy link
    Contributor Author

    @erlend-aasland erlend-aasland commented May 28, 2020

    CheckTraceCallbackContent() in [Lib/sqlite3/test/hooks.py](https://github.com/python/cpython/blob/main/Lib/sqlite3/test/hooks.py) fails for SQLite versions 3.7.3 through 3.7.14.1, apparently because of an SQLite bug that was fixed in 3.7.15 (2012-12-12). Extract from the changelog https://sqlite.org/changes.html:

    Avoid invoking the sqlite3_trace() callback multiple times when a statement is automatically reprepared due to SQLITE_SCHEMA errors.
    

    Either we fix tests for SQLite < 3.7.15, or we drop support for SQLite < 3.7.15. (I'm +1 for the latter.)

    NB: Versions pre 3.7.3 does not build at all, because of sqlite3_create_function_v2(). See bpo-40744 and #64529.

    @erlend-aasland erlend-aasland added 3.10 3.8 3.9 stdlib labels May 28, 2020
    @erlend-aasland
    Copy link
    Contributor Author

    @erlend-aasland erlend-aasland commented May 28, 2020

    Also related to bpo-26187

    @tiran
    Copy link
    Member

    @tiran tiran commented May 28, 2020

    Could you please check https://distrowatch.com/ and verify that all major Linux distributions have a sufficient version of sqlite. Please also check LTS like Debian oldstable, Ubuntu, and CentOS.

    @erlend-aasland
    Copy link
    Contributor Author

    @erlend-aasland erlend-aasland commented May 28, 2020

    Debian oldstable (jessie) has sqlite 3.8.7.1
    Ubuntu 14.04 LTS (trusty) has sqlite 3.8.2
    CentOS 7 and 8 has sqlite versions > 3.7.15

    The following distributions include sqlite version _less than_ 3.7.15:
    • Porteus (release v2.1 and older)
    • OLPC OS
    • HardenedBSD
    • 0Linux / FreePBX (release v10 and older)
    • ToOpPy Linux
    • Baruwa Enterprise Edition

    The other hits were inactive distros.

    https://distrowatch.com/search.php?pkg=sqlite&relation=less&pkgver=3.7.15&distrorange=InLatest#pkgsearch

    @berkerpeksag
    Copy link
    Member

    @berkerpeksag berkerpeksag commented May 28, 2020

    +1 for dropping support for < 3.7.15 in master. We should fix or skip tests in maintenance branches.

    @berkerpeksag berkerpeksag added type-feature labels May 28, 2020
    @erlend-aasland
    Copy link
    Contributor Author

    @erlend-aasland erlend-aasland commented May 29, 2020

    All right, will do, Berker Persag.

    @erlend-aasland
    Copy link
    Contributor Author

    @erlend-aasland erlend-aasland commented May 29, 2020

    FYI, the #if in line 1563 in [Modules/_sqlite/connection.c](https://github.com/python/cpython/blob/main/Modules/_sqlite/connection.c) is wrong, but the comment five lines below is correct:

    #if SQLITE_VERSION_NUMBER > 3007015
    PyErr_SetString(pysqlite_OperationalError, sqlite3_errstr(rc));
    #else
    switch (rc) {
    case SQLITE_ERROR:
    /* Description of SQLITE_ERROR in SQLite 3.7.14 and older
    releases. */
    PyErr_SetString(pysqlite_OperationalError,
    "SQL logic error or missing database");
    break;
    case SQLITE_READONLY:
    PyErr_SetString(pysqlite_OperationalError,
    "attempt to write a readonly database");
    break;
    case SQLITE_BUSY:
    PyErr_SetString(pysqlite_OperationalError, "database is locked");
    break;
    case SQLITE_LOCKED:
    PyErr_SetString(pysqlite_OperationalError,
    "database table is locked");
    break;
    default:
    PyErr_Format(pysqlite_OperationalError,
    "unrecognized error code: %d", rc);
    break;
    }
    #endif

    sqlite3_errstr() was introduced in 3.7.15, so the #if comparison operator should have been '>=', not '>'.

    @miss-islington
    Copy link
    Contributor

    @miss-islington miss-islington commented Jan 4, 2021

    New changeset f7f0ed5 by Erlend Egeberg Aasland in branch 'master':
    bpo-40810: Fix CheckTraceCallbackContent for SQLite pre 3.7.15 (GH-20530)
    f7f0ed5

    @gvanrossum
    Copy link
    Member

    @gvanrossum gvanrossum commented Jan 4, 2021

    Please ping this issue if the backports don't auto-merge after the tests pass, or if the tests fail.

    @erlend-aasland
    Copy link
    Contributor Author

    @erlend-aasland erlend-aasland commented Jan 4, 2021

    +1 for dropping support for < 3.7.15 in master. We should fix or skip tests in maintenance branches.

    PR's for fixing the tests in 3.9 and 3.8 are now awaiting merge. PR for dropping support for SQLite pre 3.7.15 in master is opened.

    @miss-islington
    Copy link
    Contributor

    @miss-islington miss-islington commented Jan 4, 2021

    New changeset 0ccac5f by Miss Islington (bot) in branch '3.8':
    bpo-40810: Fix CheckTraceCallbackContent for SQLite pre 3.7.15 (GH-20530)
    0ccac5f

    @miss-islington
    Copy link
    Contributor

    @miss-islington miss-islington commented Jan 4, 2021

    New changeset 80e5732 by Miss Islington (bot) in branch '3.9':
    bpo-40810: Fix CheckTraceCallbackContent for SQLite pre 3.7.15 (GH-20530)
    80e5732

    @brandtbucher
    Copy link
    Member

    @brandtbucher brandtbucher commented Jan 5, 2021

    It looks like the markup in the NEWS entry broke Travis on master. I guess that's one downside of Travis not being a required job anymore.

    @berkerpeksag
    Copy link
    Member

    @berkerpeksag berkerpeksag commented Jan 6, 2021

    New changeset cf0b239 by Erlend Egeberg Aasland in branch 'master':
    bpo-40810: Require SQLite 3.7.15 (GH-24106)
    cf0b239

    @berkerpeksag
    Copy link
    Member

    @berkerpeksag berkerpeksag commented Jan 6, 2021

    I think this is now done. Thank you, Erlend!

    @berkerpeksag berkerpeksag removed the 3.7 label Jan 6, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 3.9 3.10 stdlib type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants