-
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
planner, statistics: use the correct column ID when recording stats loading status #52208
Changes from all commits
c35daf5
e60c752
72dfc6a
33291ed
a44e2b2
93eb1e6
51918df
a7eab99
9808c88
a320f42
65c5878
6758bd7
688b40d
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 |
---|---|---|
|
@@ -139,7 +139,7 @@ func crossEstimateRowCount(sctx context.PlanContext, | |
if col == nil || len(path.AccessConds) > 0 { | ||
return 0, false, corr | ||
} | ||
colID := col.UniqueID | ||
colUniqueID := col.UniqueID | ||
if corr < 0 { | ||
desc = !desc | ||
} | ||
|
@@ -152,11 +152,11 @@ func crossEstimateRowCount(sctx context.PlanContext, | |
return 0, err == nil, corr | ||
} | ||
idxID := int64(-1) | ||
idxIDs, idxExists := dsStatsInfo.HistColl.ColID2IdxIDs[colID] | ||
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 is better to use the clearly name 'colUniqueID' instead of 'colID' to avoid the wrong used. |
||
idxIDs, idxExists := dsStatsInfo.HistColl.ColUniqueID2IdxIDs[colUniqueID] | ||
if idxExists && len(idxIDs) > 0 { | ||
idxID = idxIDs[0] | ||
} | ||
rangeCounts, ok := getColumnRangeCounts(sctx, colID, ranges, dsTableStats.HistColl, idxID) | ||
rangeCounts, ok := getColumnRangeCounts(sctx, colUniqueID, ranges, dsTableStats.HistColl, idxID) | ||
if !ok { | ||
return 0, false, corr | ||
} | ||
|
@@ -168,7 +168,7 @@ func crossEstimateRowCount(sctx context.PlanContext, | |
if idxExists { | ||
rangeCount, err = GetRowCountByIndexRanges(sctx, dsTableStats.HistColl, idxID, convertedRanges) | ||
} else { | ||
rangeCount, err = GetRowCountByColumnRanges(sctx, dsTableStats.HistColl, colID, convertedRanges) | ||
rangeCount, err = GetRowCountByColumnRanges(sctx, dsTableStats.HistColl, colUniqueID, convertedRanges) | ||
} | ||
if err != nil { | ||
return 0, false, corr | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,23 +33,27 @@ func init() { | |
} | ||
|
||
// GetRowCountByColumnRanges estimates the row count by a slice of Range. | ||
func GetRowCountByColumnRanges(sctx context.PlanContext, coll *statistics.HistColl, colID int64, colRanges []*ranger.Range) (result float64, err error) { | ||
func GetRowCountByColumnRanges(sctx context.PlanContext, coll *statistics.HistColl, colUniqueID int64, colRanges []*ranger.Range) (result float64, err error) { | ||
var name string | ||
if sctx.GetSessionVars().StmtCtx.EnableOptimizerDebugTrace { | ||
debugtrace.EnterContextCommon(sctx) | ||
debugTraceGetRowCountInput(sctx, colID, colRanges) | ||
debugTraceGetRowCountInput(sctx, colUniqueID, colRanges) | ||
defer func() { | ||
debugtrace.RecordAnyValuesWithNames(sctx, "Name", name, "Result", result) | ||
debugtrace.LeaveContextCommon(sctx) | ||
}() | ||
} | ||
sc := sctx.GetSessionVars().StmtCtx | ||
c, ok := coll.Columns[colID] | ||
recordUsedItemStatsStatus(sctx, c, coll.PhysicalID, colID) | ||
c, ok := coll.Columns[colUniqueID] | ||
colInfoID := colUniqueID | ||
if len(coll.UniqueID2colInfoID) > 0 { | ||
colInfoID = coll.UniqueID2colInfoID[colUniqueID] | ||
} | ||
recordUsedItemStatsStatus(sctx, c, coll.PhysicalID, colInfoID) | ||
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. Maybe we should distinguish colId and colUniqueId in explain such as "colID: xxx" , "colUniqueID: XXX" 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. We want the column name in the EXPLAIN result, which should be fetched using the column ID from the metadata instead of the |
||
if c != nil && c.Info != nil { | ||
name = c.Info.Name.O | ||
} | ||
if statistics.ColumnStatsIsInvalid(c, sctx, coll, colID) { | ||
if statistics.ColumnStatsIsInvalid(c, sctx, coll, colUniqueID) { | ||
result, err = getPseudoRowCountByColumnRanges(sc.TypeCtx(), float64(coll.RealtimeCount), colRanges, 0) | ||
if err == nil && sc.EnableOptimizerCETrace && ok { | ||
ceTraceRange(sctx, coll.PhysicalID, []string{c.Info.Name.O}, colRanges, "Column Stats-Pseudo", uint64(result)) | ||
|
@@ -71,23 +75,27 @@ func GetRowCountByColumnRanges(sctx context.PlanContext, coll *statistics.HistCo | |
} | ||
|
||
// GetRowCountByIntColumnRanges estimates the row count by a slice of IntColumnRange. | ||
func GetRowCountByIntColumnRanges(sctx context.PlanContext, coll *statistics.HistColl, colID int64, intRanges []*ranger.Range) (result float64, err error) { | ||
func GetRowCountByIntColumnRanges(sctx context.PlanContext, coll *statistics.HistColl, colUniqueID int64, intRanges []*ranger.Range) (result float64, err error) { | ||
var name string | ||
if sctx.GetSessionVars().StmtCtx.EnableOptimizerDebugTrace { | ||
debugtrace.EnterContextCommon(sctx) | ||
debugTraceGetRowCountInput(sctx, colID, intRanges) | ||
debugTraceGetRowCountInput(sctx, colUniqueID, intRanges) | ||
defer func() { | ||
debugtrace.RecordAnyValuesWithNames(sctx, "Name", name, "Result", result) | ||
debugtrace.LeaveContextCommon(sctx) | ||
}() | ||
} | ||
sc := sctx.GetSessionVars().StmtCtx | ||
c, ok := coll.Columns[colID] | ||
recordUsedItemStatsStatus(sctx, c, coll.PhysicalID, colID) | ||
c, ok := coll.Columns[colUniqueID] | ||
colInfoID := colUniqueID | ||
if len(coll.UniqueID2colInfoID) > 0 { | ||
colInfoID = coll.UniqueID2colInfoID[colUniqueID] | ||
} | ||
recordUsedItemStatsStatus(sctx, c, coll.PhysicalID, colInfoID) | ||
if c != nil && c.Info != nil { | ||
name = c.Info.Name.O | ||
} | ||
if statistics.ColumnStatsIsInvalid(c, sctx, coll, colID) { | ||
if statistics.ColumnStatsIsInvalid(c, sctx, coll, colUniqueID) { | ||
if len(intRanges) == 0 { | ||
return 0, 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.
I always want to do this, but I can never convince myself. 😃
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.
😂 I think performance benefits and memory efficiency from this rule are small. So I think it's totally OK to disable it when it causes problems or inconvenience.