Skip to content

Commit be68b3c

Browse files
author
Ole John Aske
committed
Bug#34388068 NDB: Update to char data type columns in primay keys are lost on replica
Changes to character data type column values might not be performed on the Mysql replicated slaves if: - The column value is changed to a value being binary different from the 'before'-value, but still 'equal' according to the comparison rules existing for the specific data type / collation. E.g. -- Trailing spaces removed in a varchar -- Change between upper/lower case in a case insensitive collation. AND - All columns in the specific table are declared as being part of the primary key. Root cause for this bug is that such changes to any primary key values were not contained in UPDATE triggers and the NdbEventsOperations used to propogate changes into the binlog, such that the replica could be updated. There seems to have been an underlying assumption that primary keys are 'constant', thus the NdbEventOperations API were not able to receive changes in the primary key values when unpacking the received BEFORE/AFTER values. Neither did the TUP block include and primary key changes in an UPDATE trigger - Actually it explicitely eliminated the primary key attributes from the 'changeMask' of attributes being changed. Patch consist of 2 changes: 1) DbtupTrigger sets up the mask of attributes to monitor, and later include in the BEFORE/AFTER values in an update trigger. It used to make the asumption that PK values were 'constant', such that it explicitely eliminated the PK attribute from the 'attrbiteMask' to monitor (and send). For an UPDATE trigger we now include the PK attributes as well and include them in the BEFORE values in the update trigger if some key values actually changed. (Note that the UPDATE-AFTER values are always reflected in the KEY) 2) NdbEventOperationImpl::receive_event() receive the BEFORE and AFTER image in an update-event. It would skip over any attribute values received where a NdbRecAttr was not found to receive the value into, which also included any PK values sent as part of the BEFORE/AFTER values. This code has now been enhanced to also receive PK values being part of the BEFORE values. Online upgrade considerations: As the existing BEFORE/AFTER-receive_event() code just skipped over attributes not being expected. Thus sending BEFORE_PK values from a data node with the modfied DbtupTrigger code will only result in the PK being ignored. ( -> Behave just as before) The other way around will be just the same. Thus it should be perfectly safe to run with a mix of (non-)upgraded clients and data nodes. Other changes in patch: Introduced the pkAttributeMask which is a bitmap of the PK columns in a table. That made the method Dbtup::primaryKey() obsolete and simplified some code where we previously had to loop over all attributes in a table definition whenever include/exclude of the PK columns were needed. Also simplified the code setting up the 'numAttrsToRead' in Dbtup::readTriggerInfo(). Calls to setAttrIds() is moved out of a composite if .. else code block to a common place later. The testcase 'test_event -n PrimaryKeyUpdates' has been added as part of this fix, as well as extensions to the MTR test cases ndb_rpl_cmp_varchar_charset.test and ndb_rpl_rowsize30000.test. Change-Id: Ic5bb6dca80aa94a8cca4570c6d87c6c1639b5387 (cherry picked from commit 15d0a148071bc53f6984d3f031b723ae21a13595)
1 parent 2a29d3b commit be68b3c

File tree

10 files changed

+1702
-143
lines changed

10 files changed

+1702
-143
lines changed

mysql-test/suite/ndb_rpl/r/ndb_rpl_cmp_varchar_charset.result

Lines changed: 695 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
2+
##############################
3+
# Include file used by test case forbug#34388068
4+
5+
--echo
6+
--echo
7+
--echo #####################################
8+
9+
show variables like 'ndb_log_update_%';
10+
11+
--echo #####################################
12+
--echo ############## Varchar ##############
13+
14+
--connection master
15+
create table t1(str varchar(32) primary key) engine = ndb;
16+
17+
--echo Insert space padded row, then update it to non-padded
18+
insert into t1(str) values ("one ");
19+
update t1 set str="one";
20+
21+
--echo Verify the replica update, no trailing spaces anymore!
22+
--sync_slave_with_master
23+
--connection slave
24+
select concat("<<<",str,">>>") from t1;
25+
26+
--connection master
27+
drop table t1;
28+
29+
--echo #########################################
30+
--echo ############## Longvarchar ##############
31+
32+
--connection master
33+
create table t1(str varchar(512) primary key) engine = ndb;
34+
35+
--echo Insert space padded row, then update it to non-padded
36+
insert into t1(str) values ("one ");
37+
update t1 set str="one";
38+
39+
--echo Verify the replica update, no trailing spaces anymore!
40+
--sync_slave_with_master
41+
--connection slave
42+
select concat("<<<",str,">>>") from t1;
43+
44+
--connection master
45+
drop table t1;
46+
47+
--echo #####################################
48+
--echo ############ Varbinary ##############
49+
# Varbinary not affected by bug as the binary representation
50+
# compared as non equal from the beginning.
51+
--connection master
52+
create table t1(str varbinary(32) primary key) engine = ndb;
53+
54+
--echo Insert space padded row, then update it to non-padded
55+
insert into t1(str) values ("one ");
56+
update t1 set str="one";
57+
58+
--echo Verify the replica update, no trailing spaces anymore!
59+
--sync_slave_with_master
60+
--connection slave
61+
select concat("<<<",str,">>>") from t1;
62+
63+
--connection master
64+
drop table t1;
65+
66+
# Note that fixed-char data type is not used in the above
67+
# test cases as fixed-chars are always stored space padded.
68+
# Thus, updating to a non-space padded has no effect.
69+
70+
71+
--echo ##########################################################
72+
--echo # Lower and upper case are compared as equal.
73+
--echo # Binary representation are unequal though, and update
74+
--echo # Need to be propogated to replica even when the 'after' images
75+
--echo # are 'compared-equal'.
76+
--echo # Tests inserts 'xyz', then update it to 'XYZ'.
77+
--echo # Check that rows are updated on replica as well.
78+
--echo
79+
--echo #####################################
80+
--echo ############## Varchar ##############
81+
--connection master
82+
create table t1(str varchar(32) primary key) engine = ndb;
83+
84+
--echo Insert 'xyz', then update varchar(32) to 'XYZ'
85+
insert into t1(str) values ("xyz");
86+
update t1 set str="XYZ";
87+
88+
--echo Verify the replica updated varchar(32) to 'XYZ' as well
89+
--sync_slave_with_master
90+
--connection slave
91+
select str from t1;
92+
93+
--connection master
94+
drop table t1;
95+
96+
--echo ##################################
97+
--echo ############## Char ##############
98+
--connection master
99+
create table t1(str char(32) primary key) engine = ndb;
100+
101+
--echo Insert 'xyz', then update char(32) to 'XYZ'
102+
insert into t1(str) values ("xyz");
103+
update t1 set str="XYZ";
104+
105+
--echo Verify the replica updated char(32) to 'XYZ' as well
106+
--sync_slave_with_master
107+
--connection slave
108+
select str from t1;
109+
110+
--connection master
111+
drop table t1;
112+
113+
--echo ##################################
114+
--echo ############ Varbinary ###########
115+
--connection master
116+
create table t1(str varbinary(32) primary key) engine = ndb;
117+
118+
--echo Insert 'xyz', then update varbinary(32) to 'XYZ'
119+
insert into t1(str) values ("xyz");
120+
update t1 set str="XYZ";
121+
122+
--echo Verify the replica updated varbinary(32) to 'XYZ' as well
123+
--sync_slave_with_master
124+
--connection slave
125+
select str from t1;
126+
127+
--connection master
128+
drop table t1;
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
2+
--source include/have_ndb.inc
3+
--source include/have_binlog_format_mixed_or_row.inc
4+
--source suite/ndb_rpl/ndb_master-slave.inc
5+
6+
--connection master
7+
8+
--echo ##########################################
9+
--echo # bug#34388068 NDB:Update to char data type columns in
10+
--echo # primay keys are lost on replica.
11+
12+
###############################
13+
## Test all different combinations of ndb_log_update*
14+
## Change to primary key char columns should be correctly
15+
## replicated independent of these settings
16+
17+
set global ndb_log_updated_only=0;
18+
set global ndb_log_update_as_write=0;
19+
set global ndb_log_update_minimal=0;
20+
--source ndb_rpl_cmp_varchar_charset.inc
21+
22+
set global ndb_log_updated_only=1;
23+
set global ndb_log_update_as_write=0;
24+
set global ndb_log_update_minimal=0;
25+
--source ndb_rpl_cmp_varchar_charset.inc
26+
27+
set global ndb_log_updated_only=0;
28+
set global ndb_log_update_as_write=1;
29+
set global ndb_log_update_minimal=0;
30+
--source ndb_rpl_cmp_varchar_charset.inc
31+
32+
set global ndb_log_updated_only=1;
33+
set global ndb_log_update_as_write=1;
34+
set global ndb_log_update_minimal=0;
35+
--source ndb_rpl_cmp_varchar_charset.inc
36+
37+
set global ndb_log_updated_only=0;
38+
set global ndb_log_update_as_write=1;
39+
set global ndb_log_update_minimal=1;
40+
--source ndb_rpl_cmp_varchar_charset.inc
41+
42+
set global ndb_log_updated_only=1;
43+
set global ndb_log_update_as_write=1;
44+
set global ndb_log_update_minimal=1;
45+
--source ndb_rpl_cmp_varchar_charset.inc
46+
47+
--echo ############################################################
48+
--echo The two test cases below currently *fails* to replicate the
49+
--echo changes due to Bug#35450016. The incorrect result is recorded.
50+
set global ndb_log_updated_only=0;
51+
set global ndb_log_update_as_write=0;
52+
set global ndb_log_update_minimal=1;
53+
--source ndb_rpl_cmp_varchar_charset.inc
54+
55+
set global ndb_log_updated_only=1;
56+
set global ndb_log_update_as_write=0;
57+
set global ndb_log_update_minimal=1;
58+
--source ndb_rpl_cmp_varchar_charset.inc
59+
60+
## Reset to default
61+
set global ndb_log_updated_only=default;
62+
set global ndb_log_update_as_write=default;
63+
set global ndb_log_update_minimal=default;
64+
65+
--source include/rpl_end.inc

sql/ha_ndbcluster_binlog.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5423,8 +5423,7 @@ ndbcluster_create_event_ops(THD *thd, NDB_SHARE *share,
54235423
share->op= op; // assign op in NDB_SHARE
54245424

54255425
/* Check if user explicitly requires monitoring of empty updates */
5426-
if (opt_ndb_log_empty_update)
5427-
op->setAllowEmptyUpdate(true);
5426+
op->setAllowEmptyUpdate(opt_ndb_log_empty_update);
54285427

54295428
if (op->execute())
54305429
{

storage/ndb/src/kernel/blocks/dbtup/Dbtup.hpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright (c) 2003, 2021, Oracle and/or its affiliates.
2+
Copyright (c) 2003, 2023, Oracle and/or its affiliates.
33
44
This program is free software; you can redistribute it and/or modify
55
it under the terms of the GNU General Public License, version 2.0,
@@ -52,6 +52,7 @@
5252

5353
#define JAM_FILE_ID 414
5454

55+
typedef Bitmask<MAXNROFATTRIBUTESINWORDS> AttributeMask;
5556

5657
extern EventLogger* g_eventLogger;
5758

@@ -979,7 +980,7 @@ struct TupTriggerData {
979980
* Attribute mask, defines what attributes are to be monitored
980981
* Can be seen as a compact representation of SQL column name list
981982
*/
982-
Bitmask<MAXNROFATTRIBUTESINWORDS> attributeMask;
983+
AttributeMask attributeMask;
983984

984985
/**
985986
* Next ptr (used in pool/list)
@@ -1038,8 +1039,14 @@ TupTriggerData_pool c_triggerPool;
10381039
tuxCustomTriggers(triggerPool)
10391040
{}
10401041

1041-
Bitmask<MAXNROFATTRIBUTESINWORDS> notNullAttributeMask;
1042-
Bitmask<MAXNROFATTRIBUTESINWORDS> blobAttributeMask;
1042+
AttributeMask notNullAttributeMask;
1043+
AttributeMask blobAttributeMask;
1044+
/*
1045+
Mask of primary key attributes, resp. 'all' and the subset
1046+
not being character data types. (No collation aware compare.)
1047+
*/
1048+
AttributeMask allPkAttributeMask;
1049+
AttributeMask nonCharPkAttributeMask;
10431050

10441051
/*
10451052
Extra table descriptor for dynamic attributes, or RNIL if none.
@@ -1788,10 +1795,10 @@ struct KeyReqStruct {
17881795
} m_var_data[2];
17891796

17901797
/*
1791-
* A bit mask where a bit set means that the update or insert
1792-
* was updating this record.
1798+
* A bitmap where a set bit means that the operation has
1799+
* supplied a value for this column
17931800
*/
1794-
Bitmask<MAXNROFATTRIBUTESINWORDS> changeMask;
1801+
AttributeMask changeMask;
17951802
Uint16 var_pos_array[2*MAX_ATTRIBUTES_IN_TABLE + 1];
17961803
OperationrecPtr prevOpPtr;
17971804
};
@@ -3014,19 +3021,17 @@ struct TupHeadInfo {
30143021
Uint32* beforeBuffer,
30153022
Uint32& noBeforeWords,
30163023
bool disk);
3017-
3024+
30183025
void sendTrigAttrInfo(Signal* signal,
30193026
Uint32* data,
30203027
Uint32 dataLen,
30213028
bool executeDirect,
30223029
BlockReference receiverReference);
30233030

3024-
Uint32 setAttrIds(Bitmask<MAXNROFATTRIBUTESINWORDS>& attributeMask,
3031+
Uint32 setAttrIds(const AttributeMask& attributeMask,
30253032
Uint32 noOfAttributes,
30263033
Uint32* inBuffer);
30273034

3028-
bool primaryKey(Tablerec* const, Uint32);
3029-
30303035
// these set terrorCode and return non-zero on error
30313036

30323037
int executeTuxInsertTriggers(Signal* signal,

storage/ndb/src/kernel/blocks/dbtup/DbtupMeta.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright (c) 2003, 2021, Oracle and/or its affiliates.
2+
Copyright (c) 2003, 2023, Oracle and/or its affiliates.
33
44
This program is free software; you can redistribute it and/or modify
55
it under the terms of the GNU General Public License, version 2.0,
@@ -1641,6 +1641,8 @@ Dbtup::computeTableMetaData(Tablerec *regTabPtr)
16411641
Uint32 dynamic_count= 0;
16421642
regTabPtr->blobAttributeMask.clear();
16431643
regTabPtr->notNullAttributeMask.clear();
1644+
regTabPtr->allPkAttributeMask.clear();
1645+
regTabPtr->nonCharPkAttributeMask.clear();
16441646
for (Uint32 i = 0; i < NO_DYNAMICS; ++i)
16451647
{
16461648
bzero(regTabPtr->dynVarSizeMask[i], dyn_null_words[i]<<2);
@@ -1665,11 +1667,21 @@ Dbtup::computeTableMetaData(Tablerec *regTabPtr)
16651667
jam();
16661668
regTabPtr->blobAttributeMask.set(i);
16671669
}
1668-
if(!AttributeDescriptor::getNullable(attrDescriptor))
1670+
if (!AttributeDescriptor::getNullable(attrDescriptor))
16691671
{
16701672
jam();
16711673
regTabPtr->notNullAttributeMask.set(i);
16721674
}
1675+
if (AttributeDescriptor::getPrimaryKey(attrDescriptor))
1676+
{
1677+
jam();
1678+
regTabPtr->allPkAttributeMask.set(i);
1679+
if (!AttributeOffset::getCharsetFlag(attrDes2))
1680+
{
1681+
jam();
1682+
regTabPtr->nonCharPkAttributeMask.set(i);
1683+
}
1684+
}
16731685
if (!AttributeDescriptor::getDynamic(attrDescriptor))
16741686
{
16751687
if(arr == NDB_ARRAYTYPE_FIXED || ind==DD)

0 commit comments

Comments
 (0)