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

fix: executemany() should return every executed query result #551

Closed
wants to merge 11 commits into from

Conversation

IlyaFaer
Copy link
Contributor

@IlyaFaer IlyaFaer commented Nov 3, 2020

Fixes: #518

@IlyaFaer IlyaFaer added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. api: spanner Issues related to the googleapis/python-spanner-django API. labels Nov 3, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 3, 2020
@IlyaFaer IlyaFaer marked this pull request as ready for review November 3, 2020 10:18
@IlyaFaer IlyaFaer requested a review from a team as a code owner November 3, 2020 10:18
Copy link
Contributor

@c24t c24t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments about setting the result set and row count.

Chaining the result sets from each execute call looks sane to me, but FWIW PEP 249 makes it sound like this isn't the typical use:

Use of this method for an operation which produces one or more result sets constitutes undefined behavior, and the implementation is permitted (but not required) to raise an exception when it detects that a result set has been created by an invocation of the operation.

executemany shouldn't allow DDL right? I ask because it looks like execute queues up DDL statements without executing them and only calls update_ddl on the next non-DDL call. Also sqlite disallows this.

google/cloud/spanner_dbapi/cursor.py Outdated Show resolved Hide resolved
google/cloud/spanner_dbapi/cursor.py Outdated Show resolved Hide resolved
tests/system/test_system.py Outdated Show resolved Hide resolved
@IlyaFaer
Copy link
Contributor Author

executemany shouldn't allow DDL right? I ask because it looks like execute queues up DDL statements without executing them and only calls update_ddl on the next non-DDL call

@c24t, okay, fixed.

@c24t
Copy link
Contributor

c24t commented Dec 7, 2020

Closed in favor of googleapis/python-spanner#181.

@c24t c24t closed this Dec 7, 2020
@IlyaFaer IlyaFaer deleted the stream_many branch December 7, 2020 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/python-spanner-django API. cla: yes This human has signed the Contributor License Agreement. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cursor.executemany() hiding results up to the last one
2 participants