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

*: remove non-prepared plan cache #7040

Merged
merged 3 commits into from
Jul 13, 2018
Merged

Conversation

coocood
Copy link
Member

@coocood coocood commented Jul 12, 2018

What have you changed? (mandatory)

Remove non-prepared plan cache。

What is the type of the changes? (mandatory)

  • Improvement (non-breaking change which is an improvement to an existing feature)

non-prepared plan cache is not useable if we can only do full string match.
And further development doesn't worth the effort.

How has this PR been tested? (mandatory)

unit test

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

Yes

Does this PR affect tidb-ansible update? (mandatory)

Yes

Does this PR need to be added to the release notes? (mandatory)

release note:
Remove plan-cache config option.

non-prepared plan cache is not useable if we can only do full string match.
And further development doesn't worth the effort.
@coocood
Copy link
Member Author

coocood commented Jul 12, 2018

@zz-jason @tiancaiamao PTAL

@coocood
Copy link
Member Author

coocood commented Jul 12, 2018

/run-all-tests

if recordSets, err = s.executeStatement(ctx, connID, stmtNode, stmt, recordSets); err != nil {
return nil, errors.Trace(err)
}
// Step4: Execute the physical plan.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is "step3" now.

@@ -1,58 +0,0 @@
// Copyright 2017 PingCAP, Inc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be useful, maybe we can save it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can always find the code from git history when we need it.

@coocood
Copy link
Member Author

coocood commented Jul 13, 2018

@zz-jason PTAL

@zz-jason
Copy link
Member

LGTM

@coocood
Copy link
Member Author

coocood commented Jul 13, 2018

@tiancaiamao PTAL

@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao
Copy link
Contributor

/run-all-tests

@tiancaiamao tiancaiamao added the status/LGT2 Indicates that a PR has LGTM 2. label Jul 13, 2018
@zz-jason
Copy link
Member

@coocood Please add the "release-note" label
cc @shenli

@tiancaiamao
Copy link
Contributor

And update the document too.

1 similar comment
@tiancaiamao
Copy link
Contributor

And update the document too.

@coocood coocood added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 13, 2018
@ciscoxll
Copy link
Contributor

/run-all-tests

@ngaut ngaut merged commit 63c4562 into pingcap:master Jul 13, 2018
@coocood
Copy link
Member Author

coocood commented Jul 13, 2018

/run-unit-test

@coocood coocood deleted the remove-plan-cache branch July 13, 2018 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants