executor: case refactor for point execution #12486
Merged
Merged
Conversation
Codecov Report
@@ Coverage Diff @@
## master #12486 +/- ##
===========================================
Coverage 79.8757% 79.8757%
===========================================
Files 460 460
Lines 102409 102409
===========================================
Hits 81800 81800
Misses 14691 14691
Partials 5918 5918 |
Contributor
Author
|
/run-all-tests |
jackysp
reviewed
Sep 30, 2019
jackysp
reviewed
Sep 30, 2019
| // NewPlanBuilder creates a new PlanBuilder. | ||
| func NewPlanBuilder(sctx sessionctx.Context, is infoschema.InfoSchema, processor *BlockHintProcessor) *PlanBuilder { | ||
| sctx.GetSessionVars().PlannerSelectBlockAsName = make([]model.CIStr, processor.MaxSelectStmtOffset()+1) | ||
| if processor == nil { |
Contributor
Author
There was a problem hiding this comment.
#12358 add a new feature to resolve column name, and this change need planBuilder to pass in valid hint processor. The earlier point execution optimization pass in nil hint processor and skip related logic, and the unit test case does not cover this path, so after #12358 merged, sysbench will panic. #12482 is already fixing this, this pull request make unit test cover these paths
coocood
reviewed
Sep 30, 2019
Contributor
Author
Member
|
LGTM |
winoros
approved these changes
Sep 30, 2019
coocood
added
the
status/can-merge
Contributor
|
Your auto merge job has been accepted, waiting for 12496 |
Contributor
|
/run-all-tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
former point execution case does not cover path completely, some incompatibilities are not raised until running benchmarks, refactor related unit tests.
What is changed and how it works?
there's some change from planner: deal with the case the blockProcessor is nil #12482 to verify planner: deal with the case the blockProcessor is nil #12482 change has solved the problem(planner: deal with the case the blockProcessor is nil #12482 not merged yet)
add schema check into
IsCachedExecOk. Point update under unique index situation, ddl likedrop unique indexwill lead toCachedPlannil inCachedExecutionCheck List
Tests
Code changes
Side effects
Related changes
Release note