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

feat: session.Execute without TxControl (default - serializableRW) #906

Closed
wants to merge 0 commits into from

Conversation

Albitko
Copy link

@Albitko Albitko commented Dec 10, 2023

Fix Issue : #694
@asmyasnikov I hope I understood you correctly

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Now session.Execute has following interface

Execute(
  ctx context.Context,
  tx *TransactionControl,
  query string,
  params *QueryParameters,
  opts ...options.ExecuteDataQueryOption,
) (txr Transaction, r result.Result, err error)

Issue Number: #694

What is the new behavior?

session.Execute interface will look like

Execute(
  ctx context.Context,
  query string,
  params *QueryParameters,
  opts ...options.ExecuteDataQueryOption,
) (txr Transaction, r result.Result, err error)

by default TransactionControl will be set to serializableRW
If a different TransactionControl value needs to be set,
it can be set using the WithTxControl function through the context

example:

_, res, err := s.Execute(table.WithTxControl(ctx, readTx), query, nil)

Other information

It seems this might break backwards compatibility and I'm not sure about the next version in the CHANGELOG.md.

@asmyasnikov
Copy link
Member

It is breaking change =(
I think this PR I will not merge into master on v3
But I will use your idea for next service (Query) and in Table v4

@Albitko
Copy link
Author

Albitko commented Dec 10, 2023

It is breaking change =(
I think this PR I will not merge into master on v3
But I will use your idea for next service (Query) and in Table v4

I have no idea how to remove TxControl from the Execute interface and maintain compatibility.

Was this change intended in some other way or is the issue not relevant yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants