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

Cursor.executemany() hiding results up to the last one #518

Closed
IlyaFaer opened this issue Sep 22, 2020 · 4 comments
Closed

Cursor.executemany() hiding results up to the last one #518

IlyaFaer opened this issue Sep 22, 2020 · 4 comments
Assignees
Labels
api: spanner Issues related to the googleapis/python-spanner-django API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@IlyaFaer
Copy link
Contributor

IlyaFaer commented Sep 22, 2020

I think we have a bug in Cursor.executemany() method. Given the code it runs Cursor.execute() in a cycle, while execute() is actually replacing result of every previous call with a result from the last one.

So, if we, for example, will call executemany() with several reads, we'll get only results of the last one:

Безымянный

But in fact both reads, with SingerId=12 and with SingerId=15, were executed and we got results.

@c24t, we already have a proposition of how to deal with executemany() in the design doc, PTAL at StreamedManyResultSets section.

@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 Sep 22, 2020
@IlyaFaer IlyaFaer self-assigned this Sep 22, 2020
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Sep 23, 2020
@skuruppu skuruppu added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Oct 1, 2020
@skuruppu
Copy link
Contributor

skuruppu commented Oct 1, 2020

@IlyaFaer this was turning up on SLO dashboards so I put a priority on it. Please feel free to change the priority to what you want.

@c24t c24t added this to the Beta milestone Oct 22, 2020
@IlyaFaer
Copy link
Contributor Author

@c24t, are we okay to deal with it by adding the class StreamedManyResultSets, described in the design doc? I think the transactions retry is closing to the final state, so I'll do this one as well.

@c24t
Copy link
Contributor

c24t commented Oct 29, 2020

From @IlyaFaer's doc:

New StreamedManyResultSets class:
A wrap-around iterator, which iterates through several StreamedResultSet() as if they are all merged into a single iterator. This class is intended to be used by Cursor.executemany() method. After executing a query the method will save the resulting StreamedResultSet() object or int in case of an UPDATE operation into StreamedManyResultSets, before running the next query (which results will replace the results of the previous operation given the current implementation).

Thus, Cursor.executemany() will pump up all the resulting iterators or integers into a single StreamedManyResultSets iterator. While iterating through it with fetchone(), fetchmany() and fetchall(), users will be streaming through the results of every executed operation in the same order the operations were executed.

@IlyaFaer
Copy link
Contributor Author

This one was fixed in the python-spanner PR. Closing

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. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants