Commit 8fb2e5a
This is an automated cherry-pick of #10682
### What problem does this PR solve?
Issue Number: close #10680
Problem Summary:
```SQL
DROP DATABASE IF EXISTS db_1483421321;
CREATE DATABASE db_1483421321;
USE db_1483421321;
CREATE TABLE t0(c0 TINYINT, handle INT NOT NULL AUTO_INCREMENT, PRIMARY KEY(handle));
ALTER TABLE t0 SET TIFLASH REPLICA 1;
-- this SLEEP will make sure the case always reproducable, it not, make it longer
select sleep(5);
SELECT /*+ read_from_storage(tiflash[t0]) */ * FROM t0 order by 1;
SELECT /*+ read_from_storage(tikv[t0]) */ * FROM t0 order by 1;
ALTER TABLE t0 ADD COLUMN c1 DECIMAL NULL;
ALTER TABLE t0 ADD COLUMN c2 FLOAT NULL DEFAULT 0.0795439693286002;
ALTER TABLE t0 ADD COLUMN c5 DECIMAL NOT NULL;
ALTER TABLE t0 ADD COLUMN c4 DECIMAL DEFAULT 247262911;
ALTER TABLE t0 MODIFY COLUMN c2 FLOAT NOT NULL;
ALTER TABLE t0 MODIFY COLUMN c5 DECIMAL NULL;
ALTER TABLE t0 MODIFY COLUMN c1 BIGINT DEFAULT -56083770 NOT NULL;
-- first insert
INSERT IGNORE INTO t0 (c1, c2, c5, c4) VALUES (-2051270087, 0.44141045099028775, 0.0, 15523);
UPDATE t0 SET c5 = 870337888;
ALTER TABLE t0 MODIFY COLUMN c1 BIGINT NULL;
-- second insert
INSERT INTO t0 (c2, c5, c4) VALUES (0.004406799693866592, 0.8752311290235516, 14652);
---- TiFlash data inconsistent with TiKV after modifying a column from NOT NULL to NULL
-- in tiflash, handle=2, c1=0
SELECT /*+ read_from_storage(tiflash[t0]) */ * FROM t0 order by 1;
+--------+--------+-------------+--------------+-----------+-------+
| c0 | handle | c1 | c2 | c5 | c4 |
+--------+--------+-------------+--------------+-----------+-------+
| <null> | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 |
| <null> | 2 | 0 | 0.0044067996 | 1 | 14652 |
+--------+--------+-------------+--------------+-----------+-------+
-- in tikv, handle=2, c1=null
SELECT /*+ read_from_storage(tikv[t0]) */ * FROM t0 order by 1;
+--------+--------+-------------+--------------+-----------+-------+
| c0 | handle | c1 | c2 | c5 | c4 |
+--------+--------+-------------+--------------+-----------+-------+
| <null> | 1 | -2051270087 | 0.44141045 | 870337888 | 15523 |
| <null> | 2 | <null> | 0.0044067996 | 1 | 14652 |
+--------+--------+-------------+--------------+-----------+-------+
```
#### Root cause
- TiDB can omit a column in the row value when the column is NULL and
both DefaultValue and OriginDefaultValue are nil (see TiDB CanSkip).
- If TiFlash is still on the old schema (column is NOT NULL),
addDefaultValueToColumnIfPossible currently accepts the missing column
and fills defaultValueToField().
- When origin_default_value is empty, defaultValueToField() for NOT NULL
falls back to GenDefaultField (zero), so TiFlash returns 0 while TiKV
(new schema) returns NULL.
### What is changed and how it works?
```commit-message
ddl: Fix default value filling with finer granularity
- Tightens addDefaultValueToColumnIfPossible
- For NOT NULL missing columns, force_decode=false now returns false unless there is an origin default.
- This forces a schema sync instead of silently filling 0.
- force_decode=true still fills a default value (best-effort).
```
With old schema (NOT NULL, no origin default), missing column now
triggers schema sync. After schema sync, column becomes nullable, so
missing column can be decode and stored in tiflash with `NULL`, matching
TiKV.
### Check List
Tests <!-- At least one of them must be included. -->
- [x] Unit test
- [x] Integration test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No code
Side effects
- [ ] Performance regression: Consumes more CPU
- [ ] Performance regression: Consumes more Memory
- [ ] Breaking backward compatibility
Documentation
- [ ] Affects user behaviors
- [ ] Contains syntax changes
- [ ] Contains variable changes
- [ ] Contains experimental features
- [ ] Changes MySQL compatibility
### Release note
<!-- bugfix or new feature needs a release note -->
```release-note
Fixed a TiFlash/TiKV mismatch after altering a column from NOT NULL to NULL
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* More robust handling of missing and NOT NULL column values during
reads to reduce decode failures and avoid unnecessary schema-syncs.
* **Enhancements**
* Improved default and origin-default filling, plus better
primary-key/nullable handling and origin-default parsing for more
consistent decoding across storage engines.
* Optional CPU affinity control for read threads on Linux to improve
read-thread placement.
* **Tests**
* Added unit and end-to-end tests covering default filling,
origin-default usage, nullability changes, and cross-engine consistency.
* **Diagnostics**
* Docker test scripts now emit CPU topology and loader/version info for
easier debugging.
<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
Signed-off-by: JaySon-Huang <tshent@qq.com>
Co-authored-by: JaySon <tshent@qq.com>
Co-authored-by: jinhelin <linjinhe33@gmail.com>
Co-authored-by: Lloyd-Pottiger <yan1579196623@gmail.com>
1 parent 8ed9835 commit 8fb2e5a
File tree
14 files changed
+431
-52
lines changed- dbms/src
- Storages
- DeltaMerge
- ReadThread
- tests
- KVStore
- Decode
- tests
- TiDB
- Decode
- Schema
- tests
- tests
- docker
- fullstack-test2/ddl
14 files changed
+431
-52
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| 21 | + | |
21 | 22 | | |
22 | 23 | | |
23 | 24 | | |
| |||
120 | 121 | | |
121 | 122 | | |
122 | 123 | | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
123 | 150 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
28 | 31 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | 51 | | |
78 | 52 | | |
79 | 53 | | |
| |||
142 | 116 | | |
143 | 117 | | |
144 | 118 | | |
145 | | - | |
| 119 | + | |
146 | 120 | | |
147 | 121 | | |
148 | 122 | | |
| |||
Lines changed: 0 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1555 | 1555 | | |
1556 | 1556 | | |
1557 | 1557 | | |
1558 | | - | |
1559 | 1558 | | |
1560 | 1559 | | |
1561 | 1560 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
111 | 111 | | |
112 | 112 | | |
113 | 113 | | |
| 114 | + | |
114 | 115 | | |
115 | 116 | | |
116 | 117 | | |
| |||
199 | 200 | | |
200 | 201 | | |
201 | 202 | | |
| 203 | + | |
| 204 | + | |
202 | 205 | | |
203 | 206 | | |
204 | 207 | | |
| |||
Lines changed: 173 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
691 | 691 | | |
692 | 692 | | |
693 | 693 | | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
| 776 | + | |
| 777 | + | |
| 778 | + | |
| 779 | + | |
| 780 | + | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
| 788 | + | |
| 789 | + | |
| 790 | + | |
| 791 | + | |
| 792 | + | |
| 793 | + | |
| 794 | + | |
| 795 | + | |
| 796 | + | |
| 797 | + | |
| 798 | + | |
| 799 | + | |
| 800 | + | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
| 811 | + | |
| 812 | + | |
| 813 | + | |
| 814 | + | |
| 815 | + | |
| 816 | + | |
| 817 | + | |
| 818 | + | |
| 819 | + | |
| 820 | + | |
| 821 | + | |
| 822 | + | |
| 823 | + | |
| 824 | + | |
| 825 | + | |
| 826 | + | |
| 827 | + | |
| 828 | + | |
| 829 | + | |
| 830 | + | |
| 831 | + | |
| 832 | + | |
| 833 | + | |
| 834 | + | |
| 835 | + | |
| 836 | + | |
| 837 | + | |
| 838 | + | |
| 839 | + | |
| 840 | + | |
| 841 | + | |
| 842 | + | |
| 843 | + | |
| 844 | + | |
| 845 | + | |
| 846 | + | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
| 860 | + | |
| 861 | + | |
| 862 | + | |
| 863 | + | |
| 864 | + | |
| 865 | + | |
| 866 | + | |
694 | 867 | | |
695 | 868 | | |
0 commit comments