Skip to content

Commit a3c4467

Browse files
author
Mats Kindahl
committed
WL#5151: Conversion between different types when
replicating Replace c_ptr() calls with c_ptr_safe() calls to avoid valgrind warnings. Adding code to to handle the case that no metadata was present in the table map for the column. Allow first parameter to unpack_row() to be NULL, in which case no source tables is used and hence no checks nor conversions are done. Clarifying some comments and fixing documentation for unpack_row().
1 parent a22bc99 commit a3c4467

File tree

2 files changed

+46
-21
lines changed

2 files changed

+46
-21
lines changed

sql/rpl_record.cc

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,20 @@ pack_row(TABLE *table, MY_BITMAP const* cols,
150150
the various member functions of Field and subclasses expect to
151151
write.
152152
153-
The row is assumed to only consist of the fields for which the corresponding
154-
bit in bitset @c cols is set; the other parts of the record are left alone.
153+
The row is assumed to only consist of the fields for which the
154+
corresponding bit in bitset @c cols is set; the other parts of the
155+
record are left alone.
155156
156157
At most @c colcnt columns are read: if the table is larger than
157158
that, the remaining fields are not filled in.
158159
159-
@param rli Relay log info
160+
@note The relay log information can be NULL, which means that no
161+
checking or comparison with the source table is done, simply
162+
because it is not used. This feature is used by MySQL Backup to
163+
unpack a row from from the backup image, but can be used for other
164+
purposes as well.
165+
166+
@param rli Relay log info, which can be NULL
160167
@param table Table to unpack into
161168
@param colcnt Number of columns to read from record
162169
@param row_data
@@ -170,10 +177,8 @@ pack_row(TABLE *table, MY_BITMAP const* cols,
170177
171178
@retval 0 No error
172179
173-
@retval ER_NO_DEFAULT_FOR_FIELD
174-
Returned if one of the fields existing on the slave but not on the
175-
master does not have a default value (and isn't nullable)
176-
180+
@retval HA_ERR_GENERIC
181+
A generic, internal, error caused the unpacking to fail.
177182
*/
178183
#if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
179184
int
@@ -184,8 +189,8 @@ unpack_row(Relay_log_info const *rli,
184189
{
185190
DBUG_ENTER("unpack_row");
186191
DBUG_ASSERT(row_data);
192+
DBUG_ASSERT(table);
187193
size_t const master_null_byte_count= (bitmap_bits_set(cols) + 7) / 8;
188-
int error= 0;
189194

190195
uchar const *null_ptr= row_data;
191196
uchar const *pack_ptr= row_data + master_null_byte_count;
@@ -201,14 +206,21 @@ unpack_row(Relay_log_info const *rli,
201206
// The "current" null bits
202207
unsigned int null_bits= *null_ptr++;
203208
uint i= 0;
204-
table_def *tabledef;
205-
TABLE *conv_table;
206-
bool table_found= rli->get_table_data(table, &tabledef, &conv_table);
209+
table_def *tabledef= NULL;
210+
TABLE *conv_table= NULL;
211+
bool table_found= rli && rli->get_table_data(table, &tabledef, &conv_table);
207212
DBUG_PRINT("debug", ("Table data: table_found: %d, tabldef: %p, conv_table: %p",
208213
table_found, tabledef, conv_table));
209214
DBUG_ASSERT(table_found);
210-
if (!table_found)
211-
return HA_ERR_GENERIC;
215+
216+
/*
217+
If rli is NULL it means that there is no source table and that the
218+
row shall just be unpacked without doing any checks. This feature
219+
is used by MySQL Backup, but can be used for other purposes as
220+
well.
221+
*/
222+
if (rli && !table_found)
223+
DBUG_RETURN(HA_ERR_GENERIC);
212224

213225
for (field_ptr= begin_ptr ; field_ptr < end_ptr && *field_ptr ; ++field_ptr)
214226
{
@@ -221,7 +233,7 @@ unpack_row(Relay_log_info const *rli,
221233
conv_table ? conv_table->field[field_ptr - begin_ptr] : NULL;
222234
Field *const f=
223235
conv_field ? conv_field : *field_ptr;
224-
DBUG_PRINT("debug", ("Conversion %srequired for field '%s' (#%d)",
236+
DBUG_PRINT("debug", ("Conversion %srequired for field '%s' (#%ld)",
225237
conv_field ? "" : "not ",
226238
(*field_ptr)->field_name, field_ptr - begin_ptr));
227239
DBUG_ASSERT(f != NULL);
@@ -285,7 +297,7 @@ unpack_row(Relay_log_info const *rli,
285297
conv_field->val_str(&value_string);
286298
DBUG_PRINT("debug", ("Copying field '%s' of type '%s' with value '%s'",
287299
(*field_ptr)->field_name,
288-
source_type.c_ptr(), value_string.c_ptr()));
300+
source_type.c_ptr_safe(), value_string.c_ptr_safe()));
289301
#endif
290302
copy.set(*field_ptr, f, TRUE);
291303
(*copy.do_copy)(&copy);
@@ -296,7 +308,7 @@ unpack_row(Relay_log_info const *rli,
296308
(*field_ptr)->val_str(&value_string);
297309
DBUG_PRINT("debug", ("Value of field '%s' of type '%s' is now '%s'",
298310
(*field_ptr)->field_name,
299-
target_type.c_ptr(), value_string.c_ptr()));
311+
target_type.c_ptr_safe(), value_string.c_ptr_safe()));
300312
#endif
301313
}
302314

@@ -344,7 +356,7 @@ unpack_row(Relay_log_info const *rli,
344356
*master_reclength = table->s->reclength;
345357
}
346358

347-
DBUG_RETURN(error);
359+
DBUG_RETURN(0);
348360
}
349361

350362
/**

sql/rpl_utility.cc

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -582,14 +582,27 @@ can_convert_field_to(Field *field,
582582
String field_type(field_type_buf, sizeof(field_type_buf), field->charset());
583583
field->sql_type(field_type);
584584
DBUG_PRINT("enter", ("field_type: %s, target_type: %d, source_type: %d, source_metadata: 0x%x",
585-
field_type.c_ptr(), field->real_type(), source_type, metadata));
585+
field_type.c_ptr_safe(), field->real_type(), source_type, metadata));
586586
#endif
587587
/*
588588
If the real type is the same, we need to check the metadata to
589589
decide if conversions are allowed.
590590
*/
591591
if (field->real_type() == source_type)
592592
{
593+
if (metadata == 0) // Metadata can only be zero if no metadata was provided
594+
{
595+
/*
596+
If there is no metadata, we either have an old event where no
597+
metadata were supplied, or a type that does not require any
598+
metadata. In either case, conversion can be done but no
599+
conversion table is necessary.
600+
*/
601+
DBUG_PRINT("debug", ("Base types are identical, but there is no metadata"));
602+
*order_var= 0;
603+
DBUG_RETURN(true);
604+
}
605+
593606
DBUG_PRINT("debug", ("Base types are identical, doing field size comparison"));
594607
if (field->compatible_field_size(metadata, rli, mflags, order_var))
595608
DBUG_RETURN(is_conversion_ok(*order_var, rli));
@@ -816,7 +829,7 @@ table_def::compatible_with(THD *thd, Relay_log_info *rli,
816829
rli->report(ERROR_LEVEL, ER_SLAVE_CONVERSION_FAILED,
817830
ER(ER_SLAVE_CONVERSION_FAILED),
818831
col, db_name, tbl_name,
819-
source_type.c_ptr(), target_type.c_ptr());
832+
source_type.c_ptr_safe(), target_type.c_ptr_safe());
820833
return false;
821834
}
822835
}
@@ -836,7 +849,7 @@ table_def::compatible_with(THD *thd, Relay_log_info *rli,
836849
DBUG_PRINT("debug", ("Field %s - conversion required."
837850
" Source type: '%s', Target type: '%s'",
838851
tmp_table->field[col]->field_name,
839-
source_type.c_ptr(), target_type.c_ptr()));
852+
source_type.c_ptr_safe(), target_type.c_ptr_safe()));
840853
}
841854
}
842855
#endif
@@ -920,7 +933,7 @@ TABLE *table_def::create_conversion_table(THD *thd, Relay_log_info *rli, TABLE *
920933
field_def->init_for_tmp_table(type(col),
921934
max_length,
922935
decimals,
923-
maybe_null(col), // maybe_null
936+
TRUE, // maybe_null
924937
FALSE, // unsigned_flag
925938
pack_length);
926939
field_def->charset= target_table->field[col]->charset();

0 commit comments

Comments
 (0)