GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-101117: Improve accuracy of sqlite3.Cursor.rowcount docs by erlend-aasland · Pull Request #104287 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented May 8, 2023 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

The SQLite C API sqlite3_changes() can only be relied upon when the
statment has been run to completion.

  • Issue: SQLite rowcount is corrupted when combining UPDATE RETURNING w/ id IN (?) #101117

📚 Documentation preview 📚: https://cpython-previews--104287.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@erlend-aasland
pythongh-101117: Improve accuracy of sqlite3.Cursor.rowcount docs
8ad00a2
The SQLite C API sqlite3_changes() can only be relied upon when the
statment has been run to completion.
@erlend-aasland
erlend-aasland requested a review from berkerpeksag as a code owner May 8, 2023 10:16
@erlend-aasland erlend-aasland added skip news needs backport to 3.11 only security fixes labels May 8, 2023
@bedevere-bot bedevere-bot mentioned this pull request May 8, 2023
SQLite rowcount is corrupted when combining UPDATE RETURNING w/ id IN (?) #101117
Closed
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting core review labels May 8, 2023
@erlend-aasland
erlend-aasland requested a review from CAM-Gerlach May 8, 2023 10:32
@erlend-aasland

erlend-aasland commented May 9, 2023

Copy link
Copy Markdown
Contributor Author

@rt121212121, would this docs update be an improvement?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland
erlend-aasland requested a review from AlexWaygood May 9, 2023 21:31
@rt121212121

rt121212121 commented May 9, 2023

Copy link
Copy Markdown

As a developer in the Python ecosystem I have had more breaking changes than I would like in both standard library modules and core tools like the packaging tools, this is the third recently. Each of these is regrettable and incurs both unwanted work and confusion.

Documentation would be good. I would recommend that all breaking changes are clearly and explicitly documented, both in what's new and in the library documentation. I understand that you have a lot of demands on your time, so that would be appreciated.

Personally, I would break this further. I would have the rowcount error if the problem case is encountered, rather than being incorrect. But that is asking someone to do more work, and that someone is not going to be myself.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented May 10, 2023

Copy link
Copy Markdown
Contributor Author

As a developer in the Python ecosystem I have had more breaking changes than I would like in both standard library modules and core tools like the packaging tools, this is the third recently. Each of these is regrettable and incurs both unwanted work and confusion.

I understand your frustration, and thank you for speaking up. Note that we really strive to avoid these kinds of changes; backwards compatibility is a priority. Unfortunately, as I wrote on the issue, we sometimes take the wrong step. In this case, the backwards compatibility issues were not immediately obvious.

Documentation would be good. I would recommend that all breaking changes are clearly and explicitly documented, both in what's new and in the library documentation. I understand that you have a lot of demands on your time, so that would be appreciated.

All known breaking changes are always documented, both in the changelog (NEWS entries) and in What's New. We've also introduced a new category in What's New for pending breaking changes, in order to better prepare users of stuff like deprecated APIs being removed in a future version. Of course, this implies that we know that a change is a breaking change; this was unfortunately not the case for the changes that lead to your specific problem here. IMO, the best we can do now, is to improve the existing documentation, which is what I'm trying to do with this PR.

Personally, I would break this further. I would have the rowcount error if the problem case is encountered, rather than being incorrect. But that is asking someone to do more work, and that someone is not going to be myself.

That is a slight change in the existing semantics of rowcount, so we'd have to think that through very carefully, as it may be another breaking change. For the sake of stability, I'd prefer to keep the status quo, and simply document how to use the APIs.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood
AlexWaygood approved these changes May 11, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels May 11, 2023
@erlend-aasland
erlend-aasland merged commit 7470321 into python:main May 11, 2023
@miss-islington

miss-islington commented May 11, 2023

Copy link
Copy Markdown
Contributor

Thanks @erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the awaiting merge label May 11, 2023
@erlend-aasland
erlend-aasland deleted the docs/sqlite-returning-clauses-and-rowcount branch May 11, 2023 08:44
@miss-islington

miss-islington commented May 11, 2023

Copy link
Copy Markdown
Contributor

Sorry @erlend-aasland, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 7470321f8171dce96a604ba2c24176837f999ca0 3.11

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington miss-islington assigned erlend-aasland May 11, 2023
@erlend-aasland

erlend-aasland commented May 11, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the review, Alex, and thanks for your input Roger.

AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland added a commit to erlend-aasland/cpython that referenced this pull request May 11, 2023
@erlend-aasland
[3.11] pythongh-101117: Improve accuracy of sqlite3.Cursor.rowcount d…
ad6faec
…ocs (python#104287)

The SQLite C API sqlite3_changes() can only be relied upon when the
current active statement has been run to completion.
@bedevere-bot

bedevere-bot commented May 11, 2023

Copy link
Copy Markdown

GH-104381 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label May 11, 2023
erlend-aasland added a commit that referenced this pull request May 11, 2023
@erlend-aasland
[3.11] gh-101117: Improve accuracy of sqlite3.Cursor.rowcount docs (#…
7055088
…104287) (#104381)

The SQLite C API sqlite3_changes() can only be relied upon when the
current active statement has been run to completion.
carljm added a commit to carljm/cpython that referenced this pull request May 11, 2023
@carljm
Merge branch 'main' into supermon
19b3d9e
* main: (27 commits)
  pythongh-87849: fix SEND specialization family definition (pythonGH-104268)
  pythongh-101819: Adapt _io.IOBase.seek and _io.IOBase.truncate to Argument Clinic (python#104384)
  pythongh-101819: Adapt _io._Buffered* methods to Argument Clinic (python#104367)
  pythongh-101819: Refactor `_io` futher in preparation for module isolation (python#104369)
  pythongh-101819: Adapt _io.TextIOBase methods to Argument Clinic (python#104383)
  pythongh-101117: Improve accuracy of sqlite3.Cursor.rowcount docs (python#104287)
  pythonGH-92184: Convert os.altsep to '/' in filenames when creating ZipInfo objects (python#92185)
  pythongh-104357: fix inlined comprehensions that close over iteration var (python#104368)
  pythonGH-90208: Suppress OSError exceptions from `pathlib.Path.glob()` (pythonGH-104141)
  pythonGH-102181: Improve specialization stats for SEND (pythonGH-102182)
  pythongh-103000: Optimise `dataclasses.asdict` for the common case (python#104364)
  pythongh-103538: Remove unused TK_AQUA code (pythonGH-103539)
  pythonGH-87695: Fix OSError from `pathlib.Path.glob()` (pythonGH-104292)
  pythongh-104263: Rely on Py_NAN and introduce Py_INFINITY (pythonGH-104202)
  pythongh-104010: Separate and improve docs for `typing.get_origin` and `typing.get_args` (python#104013)
  pythongh-101819: Adapt _io._BufferedIOBase_Type methods to Argument Clinic (python#104355)
  pythongh-103960: Dark mode: invert image brightness (python#103983)
  pythongh-104252: Immortalize Py_EMPTY_KEYS (pythongh-104253)
  pythongh-101819: Clean up _io windows console io after pythongh-104197 (python#104354)
  pythongh-101819: Harden _io init (python#104352)
  ...
carljm added a commit to carljm/cpython that referenced this pull request May 11, 2023
@carljm
Merge branch 'main' into fixloadsuperspec
4456dd6
* main:
  pythongh-87849: fix SEND specialization family definition (pythonGH-104268)
  pythongh-101819: Adapt _io.IOBase.seek and _io.IOBase.truncate to Argument Clinic (python#104384)
  pythongh-101819: Adapt _io._Buffered* methods to Argument Clinic (python#104367)
  pythongh-101819: Refactor `_io` futher in preparation for module isolation (python#104369)
  pythongh-101819: Adapt _io.TextIOBase methods to Argument Clinic (python#104383)
  pythongh-101117: Improve accuracy of sqlite3.Cursor.rowcount docs (python#104287)
  pythonGH-92184: Convert os.altsep to '/' in filenames when creating ZipInfo objects (python#92185)
  pythongh-104357: fix inlined comprehensions that close over iteration var (python#104368)
  pythonGH-90208: Suppress OSError exceptions from `pathlib.Path.glob()` (pythonGH-104141)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag
@CAM-Gerlach CAM-Gerlach Awaiting requested review from CAM-Gerlach

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@erlend-aasland erlend-aasland

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@erlend-aasland @rt121212121 @miss-islington @bedevere-bot @AlexWaygood
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.