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: remove sqlite3_stmt_readonly() #73541

Closed
animalize mannequin opened this issue Jan 24, 2017 · 7 comments
Closed

sqlite3: remove sqlite3_stmt_readonly() #73541

animalize mannequin opened this issue Jan 24, 2017 · 7 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@animalize
Copy link
Mannequin

animalize mannequin commented Jan 24, 2017

BPO 29355
Nosy @berkerpeksag, @willingc, @animalize, @palaviv
Dependencies
  • bpo-28518: execute("begin immediate") throwing OperationalError
  • 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 2017-02-26.16:11:55.928>
    created_at = <Date 2017-01-24.04:38:13.709>
    labels = ['3.7', 'type-feature', 'library']
    title = 'sqlite3: remove sqlite3_stmt_readonly()'
    updated_at = <Date 2017-02-26.16:11:55.926>
    user = 'https://github.com/animalize'

    bugs.python.org fields:

    activity = <Date 2017-02-26.16:11:55.926>
    actor = 'berker.peksag'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-02-26.16:11:55.928>
    closer = 'berker.peksag'
    components = ['Library (Lib)']
    creation = <Date 2017-01-24.04:38:13.709>
    creator = 'malin'
    dependencies = ['28518']
    files = []
    hgrepos = []
    issue_num = 29355
    keywords = []
    message_count = 7.0
    messages = ['286130', '288248', '288258', '288387', '288397', '288409', '288600']
    nosy_count = 5.0
    nosy_names = ['palm.kevin', 'berker.peksag', 'willingc', 'malin', 'palaviv']
    pr_nums = []
    priority = 'normal'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue29355'
    versions = ['Python 3.6', 'Python 3.7']

    @animalize
    Copy link
    Mannequin Author

    animalize mannequin commented Jan 24, 2017

    In CPython 3.6.0, sqlite3 module uses sqlite3_stmt_readonly(), however this function is only available on SQLite 3.7.4+ (release on 2010-12-07).
    Then CPython 3.6.0 can not be compiled on RHEL6 (with SQLite 3.6.x), complaints: bpo-29098, bpo-29325.

    sqlite3_stmt_readonly() was introduced in 284676cf2ac8, it was used twice [1][2], but it seems that we can avoid using this function in both of them.

    [1] https://hg.python.org/cpython/file/3.6/Modules/_sqlite/cursor.c#l517
    With palaviv's patches of bpo-28518, we can eliminate sqlite3_stmt_readonly() in here.

    [2] https://hg.python.org/cpython/file/3.6/Modules/_sqlite/cursor.c#l612
    We can use the old way in here, the old way is even better IMO.

    This issue depends on bpo-28518.

    @animalize animalize mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jan 24, 2017
    @willingc
    Copy link
    Contributor

    Another report of sqlite3 not compiling correctly on RHEL6 (jupyterhub/jupyterhub#991). Is there currently a solution to build Python 3.6 with sqlite3 support?

    @animalize
    Copy link
    Mannequin Author

    animalize mannequin commented Feb 21, 2017

    Is there currently a solution to build Python 3.6 with sqlite3 support?

    I installed SQLite 3.16.2 on a Debian-based system with this way:
    http://stackoverflow.com/questions/26261080
    No idea whether it will work on RHEL 6.

    @berkerpeksag
    Copy link
    Member

    Thanks for the report, Ma.

    We can use the old way in here, the old way is even better IMO.

    What do you mean by the old way? The use of switch statement?

    @animalize
    Copy link
    Mannequin Author

    animalize mannequin commented Feb 23, 2017

    The "old way" is not using sqlite3_stmt_readonly().

    Before 3.6.0, we only count rowcount for INSERT, UPDATE, DELETE, REPLACE:
    https://hg.python.org/cpython/file/3.5/Modules/_sqlite/cursor.c#l689

    These four statements can be representd by is_dml in PR 245:
    cbeabf4#diff-5a6abb9997d51e693f3b24986659c857R88

    So only change this line is ok, then restore the behavior before 3.6.0 exactly:

    - if (!sqlite3_stmt_readonly(self->statement->st)) {
    + if (self->statement->is_dml) {
        self->rowcount += (long)sqlite3_changes(self->connection->db);
    } else {
        self->rowcount= -1L;
    }

    Why it's better?
    sqlite3_changes() only works for INSERT, UPDATE, DELETE. see:
    https://sqlite.org/c3ref/changes.html
    So using sqlite3_stmt_readonly() to determine statements is not necessary at all.

    In addition, we can add a comment for code safe in statement.c.
    + /* is_dml is used in two sites:
    + 1, determine statements for implicit BEGIN.
    + 2, determine statements for counting rowcount */
    self->is_dml = (PyOS_strnicmp(p, "insert ", 7) == 0)
    || (PyOS_strnicmp(p, "update ", 7) == 0)
    || (PyOS_strnicmp(p, "delete ", 7) == 0)
    || (PyOS_strnicmp(p, "replace ", 8) == 0);

    @berkerpeksag
    Copy link
    Member

    • if (!sqlite3_stmt_readonly(self->statement->st)) {
    • if (self->statement->is_dml) {

    Ah, yes now I understand what do you mean! :) I agree and this is already in my local branch to fix bpo-28518.

    @berkerpeksag
    Copy link
    Member

    The patch for bpo-28518 has been merged and it doesn't contain any use of sqlite3_stmt_readonly(). Closing this as 'out of date'. Thanks again Ma!

    @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.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants