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

Allow returning values from transactional functions #3493

Closed
waw-google opened this issue Jun 9, 2017 · 5 comments
Closed

Allow returning values from transactional functions #3493

waw-google opened this issue Jun 9, 2017 · 5 comments
Assignees
Labels
api: spanner Issues related to the Spanner API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@waw-google
Copy link

Somebody asks in the Slack:

Question about spanner python APIs, specifically transactions.
In a read-write transaction, is it possible to return the updated
value without hitting the database again?

let's say I have the following transaction (pretty much the example from the docs)

    second_album_result = transaction.read(
            table='Albums', columns=('MarketingBudget',),
            keyset=second_album_keyset, limit=1)
    second_album_row = list(second_album_result)[0]
    second_album_budget = second_album_row[0]
    second_album_budget += 10000000
    transaction.update(table=mytable, columns=('MarketingBudget'),
                        keyset=second_album_keyset, values=[(second_album_budget)]
                    )  
    return second_album_budget
database.run_in_transaction(f)

is the return at all possible?

I believe that this is currently not possible in the Python library, because when you pass a closure to run_in_transaction, the return value always gets ignored, and it just returns the commit timestamp.

However this is possible in the Cloud Spanner Java library, and being be able to return whatever you want from the body of a transactional function in the case that a transaction successfully commits is extremely useful.

@dhermes dhermes added the api: spanner Issues related to the Spanner API. label Jun 9, 2017
@theacodes theacodes added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. status: acknowledged type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jun 21, 2017
@bjwatson
Copy link

bjwatson commented Aug 7, 2017

@vkedia @lukesneeringer This sounds useful, but IIUC it sounds like it could be done post-Beta since changing database.run_in_transaction(f) from returning nothing to returning something is not a breaking change.

@lukesneeringer
Copy link
Contributor

It is technically breaking because we do return the commit timestamp, but given that the commit timestamp is pretty useless, I am inclined to agree.

@bjwatson
Copy link

bjwatson commented Aug 8, 2017

Thanks, @lukesneeringer. A meaningless breaking change seems fine for a Beta->GA fix, but we want to avoid any breaking changes post-GA. So before GA we need to:

  • fix this for real,
  • remove the useless commit timestamp return value, or
  • turn the return value into a dict to allow for future expansion (which seems like it might be over-engineering for this use case).

@bjwatson
Copy link

bjwatson commented Aug 8, 2017

Nevermind, just saw that you fixed this for real. Thanks, @lukesneeringer!

@tseaver
Copy link
Contributor

tseaver commented Aug 8, 2017

@lukesneeringer FWIW, the major use for the committed timestamp is in setting up subsequent snapshots using request_timestamp.

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 Spanner API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

6 participants