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

*: make SET TRANSACTION ISOLATION LEVEL READ COMMITTED take effect #3619

Merged
merged 9 commits into from Jul 17, 2017

Conversation

Projects
None yet
4 participants
@tiancaiamao
Contributor

tiancaiamao commented Jul 4, 2017

it is equivalent to setting "tx_isolation" variable.
if the value is READ-COMMITTED, set the transaction isolation level option.

@disksing @zimulala @shenli

tp.Charset, tp.Collate = parser.charset, parser.collation
expr := ast.NewValueExpr($3)
expr.SetType(tp)
$$ = &ast.VariableAssignment{Name: "tx_isolation", Value: expr, IsSystem: true}

This comment has been minimized.

@shenli

shenli Jul 4, 2017

Member

Use a const for tx_isolation.

@shenli

shenli Jul 4, 2017

Member

Use a const for tx_isolation.

This comment has been minimized.

@tiancaiamao

tiancaiamao Jul 4, 2017

Contributor

tx_isolation is a session variable name.
I don't want to put it in other places except variable package.
I also don't like parser package to import variable package.
So use the string "tx_isolation" is the best compromise.

@tiancaiamao

tiancaiamao Jul 4, 2017

Contributor

tx_isolation is a session variable name.
I don't want to put it in other places except variable package.
I also don't like parser package to import variable package.
So use the string "tx_isolation" is the best compromise.

Show outdated Hide outdated session.go
@@ -1130,6 +1130,9 @@ func (s *session) ActivePendingTxn() error {
if err != nil {
return errors.Trace(err)
}
if s.sessionVars.Systems[variable.TxnIsolation] == "READ-COMMITTED" {

This comment has been minimized.

@shenli

shenli Jul 4, 2017

Member

Use const for READ-COMMITTED.

@shenli

shenli Jul 4, 2017

Member

Use const for READ-COMMITTED.

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Jul 4, 2017

Member

Any need to print error when user wants to set unsupported levels?

Member

disksing commented Jul 4, 2017

Any need to print error when user wants to set unsupported levels?

*: make SET TRANSACTION ISOLATION LEVEL READ COMMITTED take effect
it is equivalent to setting "tx_isolation" variable.
if the value is READ-COMMITTED, set the transaction isolation level option.
@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Jul 4, 2017

Member

Need to set kv.Request too.

Member

disksing commented Jul 4, 2017

Need to set kv.Request too.

tiancaiamao added some commits Jul 4, 2017

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Jul 17, 2017

Contributor
Begin;
SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
SELECT * FROM T;    // isolation level should take effect in this query.
COMMIT;
Contributor

tiancaiamao commented Jul 17, 2017

Begin;
SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
SELECT * FROM T;    // isolation level should take effect in this query.
COMMIT;
@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
Contributor

tiancaiamao commented Jul 17, 2017

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Jul 17, 2017

Member

LGTM

Member

disksing commented Jul 17, 2017

LGTM

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Jul 17, 2017

Member

LGTM

Member

shenli commented Jul 17, 2017

LGTM

@shenli

shenli approved these changes Jul 17, 2017

@hanfei1991 hanfei1991 merged commit d8aaa22 into master Jul 17, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@hanfei1991 hanfei1991 deleted the tiancaiamao/read-committed branch Jul 17, 2017

dbjoa added a commit to cloud-pi/tidb that referenced this pull request Jul 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment