amazon-qldb-shell icon indicating copy to clipboard operation
amazon-qldb-shell copied to clipboard

2.0 - `start` vs `start transaction`

Open brack opened this issue 4 years ago • 3 comments

In the Python shell, start would start a transaction

In the Rust shell, I must type start transaction or begin

Unsure if we want to consider this a regression, but users of the Python shell switching over might find this jarring. Perhaps we should add support for start?

brack avatar Sep 08 '21 23:09 brack

also, after messing around a bit more, having to type the extra characters all the time is a little bit bothersome. not a major issue, but we should make sure to remind people about begin if we want them to use that.

e.g., in this usage error, it should also say "or '\begin'"

qldb> select * from Vehicle
usage error: No active transaction and not in auto-commit mode. Start a transaction with '\start transaction'

brack avatar Sep 09 '21 00:09 brack

There was quite a lot of discussion around this, and the break from the 1.0 syntax was intentional. We're worried about forwards compatibility (e.g. with stored procedures), and so opted for a conservative syntax. I'm not 100% sure we've made the right decision. It would be helpful to get the context (there were some email threads..) from @alpian and then we can discuss further.

marcbowes avatar Sep 09 '21 21:09 marcbowes

after getting more context from the discussion with Ian and Yilin, and given that there was a conscious decision about it (especially forward compatibility issues), it seems reasonable to stick with start transaction and begin. I don't know how much it will impact customers, although I think it could be good to have our rationale/context captured in this issue before we close it out.

i think then, my main feedback here would be to make sure we include begin in our messaging, e.g., the usage error which mentions start transaction but does not also mention begin (whereas the help messaging mentions both, etc). i can open a separate issue for this or potentially just put in a PR, which would be much lower priority than the core behavior change of no longer supporting start.

brack avatar Sep 10 '21 18:09 brack