-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
plan: remove other accessPath
s if one is unique key and full matched.
#6925
Changes from 1 commit
d2b83a6
1aea370
a352724
5ae90d3
d5058ee
baa97cf
d098c09
4593783
929fc2e
c4770a0
b71b70f
b385a40
3dd9749
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -332,7 +332,7 @@ type accessPath struct { | |
forced bool | ||
} | ||
|
||
func (ds *DataSource) deriveTablePathStats(path *accessPath) error { | ||
func (ds *DataSource) deriveTablePathStats(path *accessPath) (bool, error) { | ||
var err error | ||
sc := ds.ctx.GetSessionVars().StmtCtx | ||
path.countAfterAccess = float64(ds.statisticTable.Count) | ||
|
@@ -345,22 +345,29 @@ func (ds *DataSource) deriveTablePathStats(path *accessPath) error { | |
} | ||
if pkCol == nil { | ||
path.ranges = ranger.FullIntRange(false) | ||
return nil | ||
return false, nil | ||
} | ||
path.ranges = ranger.FullIntRange(mysql.HasUnsignedFlag(pkCol.RetType.Flag)) | ||
if len(ds.pushedDownConds) == 0 { | ||
return nil | ||
return false, nil | ||
} | ||
path.accessConds, path.tableFilters = ranger.DetachCondsForTableRange(ds.ctx, ds.pushedDownConds, pkCol) | ||
path.ranges, err = ranger.BuildTableRange(path.accessConds, sc, pkCol.RetType) | ||
if err != nil { | ||
return errors.Trace(err) | ||
return false, errors.Trace(err) | ||
} | ||
path.countAfterAccess, err = ds.statisticTable.GetRowCountByIntColumnRanges(sc, pkCol.ID, path.ranges) | ||
return errors.Trace(err) | ||
noIntervalRange := true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add comments here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about s/noIntervalRange/allPointRanges/? This variable renaming can make code logic more clear. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/noIntervalRange/allPointRanges/ or s/noIntervalRange/onlyPoint/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any example? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. like a > 5 and a < 3... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
for _, ran := range path.ranges { | ||
if !ran.IsPoint(sc) { | ||
noIntervalRange = false | ||
break | ||
} | ||
} | ||
return noIntervalRange, errors.Trace(err) | ||
} | ||
|
||
func (ds *DataSource) deriveIndexPathStats(path *accessPath) error { | ||
func (ds *DataSource) deriveIndexPathStats(path *accessPath) (bool, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
var err error | ||
sc := ds.ctx.GetSessionVars().StmtCtx | ||
path.ranges = ranger.FullRange() | ||
|
@@ -369,11 +376,11 @@ func (ds *DataSource) deriveIndexPathStats(path *accessPath) error { | |
if len(idxCols) != 0 { | ||
path.ranges, path.accessConds, path.indexFilters, path.eqCondCount, err = ranger.DetachCondAndBuildRangeForIndex(ds.ctx, ds.pushedDownConds, idxCols, lengths) | ||
if err != nil { | ||
return errors.Trace(err) | ||
return false, errors.Trace(err) | ||
} | ||
path.countAfterAccess, err = ds.statisticTable.GetRowCountByIndexRanges(sc, path.index.ID, path.ranges) | ||
if err != nil { | ||
return errors.Trace(err) | ||
return false, errors.Trace(err) | ||
} | ||
path.indexFilters, path.tableFilters = splitIndexFilterConditions(path.indexFilters, path.index.Columns, ds.tableInfo) | ||
} else { | ||
|
@@ -388,7 +395,7 @@ func (ds *DataSource) deriveIndexPathStats(path *accessPath) error { | |
} | ||
path.countAfterIndex = math.Max(path.countAfterAccess*selectivity, ds.statsAfterSelect.count) | ||
} | ||
return nil | ||
return path.index.Unique && path.eqCondCount == len(path.index.Columns), nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's better to only check whether the ranges are all point ranges, just like |
||
} | ||
|
||
func (ds *DataSource) getPKIsHandleCol() *expression.Column { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,16 +128,27 @@ func (ds *DataSource) deriveStats() (*statsInfo, error) { | |
ds.statsAfterSelect = ds.getStatsByFilter(ds.pushedDownConds) | ||
for _, path := range ds.possibleAccessPaths { | ||
if path.isTablePath { | ||
err := ds.deriveTablePathStats(path) | ||
justPointRange, err := ds.deriveTablePathStats(path) | ||
if err != nil { | ||
return nil, errors.Trace(err) | ||
} | ||
// If there's only point range. Just remove other possible paths. | ||
if justPointRange { | ||
ds.possibleAccessPaths[0] = path | ||
ds.possibleAccessPaths = ds.possibleAccessPaths[:1] | ||
break | ||
} | ||
continue | ||
} | ||
err := ds.deriveIndexPathStats(path) | ||
uniqueAndOnlyPoint, err := ds.deriveIndexPathStats(path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to tell that this key is unique key. |
||
if err != nil { | ||
return nil, errors.Trace(err) | ||
} | ||
// If there's only point range and this index is unique key. Just remove other possible paths. | ||
if uniqueAndOnlyPoint { | ||
ds.possibleAccessPaths[0] = path | ||
ds.possibleAccessPaths = ds.possibleAccessPaths[:1] | ||
} | ||
} | ||
return ds.statsAfterSelect, nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment for the newly added return value.