Skip to content

sqlite3: remove sqlite3_stmt_readonly() #73541

Description

@animalize
mannequin
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']

    Activity

    1. animalize commented on Jan 24, 2017

      animalizemannequin
      MannequinAuthor

      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.

    2. added
      stdlibStandard Library Python modules in the Lib/ directory
      type-featureA feature request or enhancement
      on Jan 24, 2017
    3. willingc commented on Feb 20, 2017

      @willingc
      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?

    4. animalize commented on Feb 21, 2017

      animalizemannequin
      MannequinAuthor

      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.

    5. berkerpeksag commented on Feb 22, 2017

      @berkerpeksag
      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?

    6. animalize commented on Feb 23, 2017

      animalizemannequin
      MannequinAuthor

      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);

    7. berkerpeksag commented on Feb 23, 2017

      @berkerpeksag
      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.

    8. berkerpeksag commented on Feb 26, 2017

      @berkerpeksag
      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!

    9. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.7 (EOL)end of lifestdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions