-
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: fix wrong selectivity for inner selection in index join #10633
planner: fix wrong selectivity for inner selection in index join #10633
Conversation
/run-all-tests |
/run-integration-common-test |
logutil.Logger(context.Background()).Warn("calculate selectivity faild, use selection factor", zap.Error(err)) | ||
selectivity = selectionFactor | ||
} | ||
path.countAfterIndex = rowCount * selectivity |
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.
How about make a new method on path
for line 596~603
and exhaust_physical_plans.go:506~513
.
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.
Hmm, these lines are simple enough and are unnecessary to be extracted?
Codecov Report
@@ Coverage Diff @@
## master #10633 +/- ##
================================================
- Coverage 80.1788% 80.1579% -0.0209%
================================================
Files 415 415
Lines 88350 88358 +8
================================================
- Hits 70838 70826 -12
- Misses 12273 12293 +20
Partials 5239 5239 |
├─IndexScan_15 10.00 cop table:rr, index:aid, dic, range: decided by [eq(test.rr.aid, test.dt.aid) eq(test.rr.dic, test.dt.dic)], keep order:false, stats:pseudo | ||
└─Selection_17 3.33 cop eq(test.rr.pt, "ios"), gt(test.rr.t, 1478185592) | ||
└─Selection_17 0.00 cop eq(test.rr.pt, "ios"), gt(test.rr.t, 1478185592) |
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.
Note: the real row count here is 0.003333333333333334
, which is reasonable; it is displayed as 0.00
because we are using count := string(strconv.AppendFloat([]byte{}, p.statsInfo().RowCount, 'f', 2, 64))
in prepareOperatorInfo
.
/run-all-tests |
50fe0e5
to
eacdab0
Compare
a920e5d
to
3046a25
Compare
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.
LGTM
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.
LGTM
What problem does this PR solve?
Fix #10630
What is changed and how it works?
ds.stats
as table plan's stats when it is double read in index join's inner side, because for inner child of index join, we are building new stats using average row count from scratch, so it is different from normalDataSource::DeriveStats
;Check List
Tests
Code changes
N/A
Side effects
N/A
Related changes