-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
*: enable the predicate columns feature by default | tidb-test=pr/2361 #54440
*: enable the predicate columns feature by default | tidb-test=pr/2361 #54440
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #54440 +/- ##
=================================================
- Coverage 72.7895% 56.8086% -15.9810%
=================================================
Files 1549 1665 +116
Lines 436285 606397 +170112
=================================================
+ Hits 317570 344486 +26916
- Misses 99067 238247 +139180
- Partials 19648 23664 +4016
Flags with carried forward coverage won't be shown. Click here to find out more.
|
b884856
to
2f38ff2
Compare
8d81dda
to
5a7a370
Compare
5a7a370
to
a275d36
Compare
6faee49
to
40ad678
Compare
1b7d9a1
to
4bb5e3c
Compare
4bb5e3c
to
f475fd9
Compare
Test locally: Upgrade
Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.
MySQL [(none)]> select @@tidb_analyze_column_options;
+-------------------------------+
| @@tidb_analyze_column_options |
+-------------------------------+
| ALL |
+-------------------------------+
1 row in set (0.00 sec) New cluster:
mysql> select @@tidb_analyze_column_options;
+-------------------------------+
| @@tidb_analyze_column_options |
+-------------------------------+
| PREDICATE |
+-------------------------------+
1 row in set (0.01 sec) |
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.
🔢 Self-check (PR reviewed by myself and ready for feedback.)
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 for DDL part
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: easonn7, GMHDBJD, Leavrth, qw4990 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
What problem does this PR solve?
Issue Number: ref #53567
Problem Summary:
What changed and how does it work?
I have enabled the predicate columns feature by default. However, this caused a lot of test cases to break because we updated them to collect predicate columns' statistics. Here's how I tackled updating these cases:
We need to find a way to collect the predicate columns without having to wait for the stats usage dumping process during integration tests. I'll address this after completing all the testing work for this feature. In the meantime, we can always use all the columns as a workaround.
At the same time, I added the upgradeToVer210 function to ensure that we don't change the behavior of columns collecting(all columns) if the users upgrade the TiDB cluster from a lower version.
You can find the benchmark result here: http://perf.pingcap.net/d/iarAjGo7z/benchbot-overview?orgId=1&var-email=example%40pingcap.cn&var-testbed=benchbot-oltp-001-tps-7598610-1-822&from=now-30d&to=now (PingCAP internally only) No regression from the benchmark result.
I will do more tests after this PR is merged.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.