feat(spanner): support max_commit_delay in Spanner transactions

[nginsberg-google]

Add maxCommitDelay to CommitOptions.

[conventional-commit-lint-gcf]

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

[rahul2393]

Please fix the tests.

[nginsberg-google]

OK, unit tests are fixed. The issue was as follows:

I had it set so that merging two commit options simply overrode the maxCommitDelay with the new maxCommitDelay. However, when we are creating requests, we sometimes merge commit options with an empty commit options and expect it to be a no-op. Merging maxCommitDelay is subtle, and I'm not sure what the right answer is. There are a couple of obvious cases of merge(clientOptions, transactionOptions):

merge(nil, nil) -> nil
merge(100, 50) -> 50.

However, what should happen in this case?:
merge(100, nil) -> ?

I can see two potential answers: 100, or nil. You could argue that the user has set the value nil on the transaction, and thus wants to remove the commitDelay, since it's an optional field. You could also argue that the usage pattern of this is likely to be that the user has set the options at the client level, and then not set anything at the transaction level, wanting the client level value to propagate. The latter seems more likely to me. It also seems silly to have merge(100, nil) -> nil, as then setting a value on the client level would be useless - you would always have to respecify it on the transaction level.

So I have decided that:
merge(100, nil) -> 100.

This means that if a user specifies a maxCommitDelay on the client level, there is no way to unspecify it. You can change it, but you can't get rid of it. If the user wants to only specify the maxCommitDelay on some transactions, they should just leave it unset at the client level, and set it on the transactions they want to turn it on for. I have captured this subtlety in another unit test.

Additionally, I changed the type of CommitOptions.MaxCommitDelay from a time.Duration to a *time.Duration, so the nil value could be properly represented.