Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion mysql-test/suite/binlog_encryption/rpl_typeconv.result
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ On_Master LONGTEXT,
On_Slave LONGTEXT,
Expected LONGTEXT,
Compare INT,
Error TEXT);
Error INT2);
SELECT @@global.slave_type_conversions;
@@global.slave_type_conversions

Expand Down Expand Up @@ -51,6 +51,10 @@ SET GLOBAL SLAVE_TYPE_CONVERSIONS='';
# MDEV-17098 DATE <-> DATETIME
#
# End of MDEV-17098
#
# MDEV-39240 Invalid / MDEV-32188-only TIMESTAMPs
#
# End of MDEV-39240
include/rpl_reset.inc
connection slave;
SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_NON_LOSSY';
Expand All @@ -67,6 +71,10 @@ SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_NON_LOSSY';
# MDEV-17098 DATE <-> DATETIME
#
# End of MDEV-17098
#
# MDEV-39240 Invalid / MDEV-32188-only TIMESTAMPs
#
# End of MDEV-39240
include/rpl_reset.inc
connection slave;
SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_LOSSY';
Expand All @@ -83,6 +91,10 @@ SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_LOSSY';
# MDEV-17098 DATE <-> DATETIME
#
# End of MDEV-17098
#
# MDEV-39240 Invalid / MDEV-32188-only TIMESTAMPs
#
# End of MDEV-39240
include/rpl_reset.inc
connection slave;
SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_LOSSY,ALL_NON_LOSSY';
Expand All @@ -99,6 +111,10 @@ SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_LOSSY,ALL_NON_LOSSY';
# MDEV-17098 DATE <-> DATETIME
#
# End of MDEV-17098
#
# MDEV-39240 Invalid / MDEV-32188-only TIMESTAMPs
#
# End of MDEV-39240
include/rpl_reset.inc
connection slave;
**** Result of conversions ****
Expand Down Expand Up @@ -267,6 +283,8 @@ DATE DATETIME(0) <Correct error>
DATETIME(6) DATE <Correct error>
DATETIME(6) DATE <Correct error>
DATETIME(0) DATE <Correct error>
TIMESTAMP(0) TIMESTAMP(0) <Correct value>
TIMESTAMP(0) TIMESTAMP(0) <Correct error>
TINYBLOB TINYBLOB ALL_NON_LOSSY <Correct value>
TINYBLOB BLOB ALL_NON_LOSSY <Correct value>
TINYBLOB MEDIUMBLOB ALL_NON_LOSSY <Correct value>
Expand Down Expand Up @@ -431,6 +449,8 @@ DATE DATETIME(0) ALL_NON_LOSSY <Correct value>
DATETIME(6) DATE ALL_NON_LOSSY <Correct error>
DATETIME(6) DATE ALL_NON_LOSSY <Correct error>
DATETIME(0) DATE ALL_NON_LOSSY <Correct error>
TIMESTAMP(0) TIMESTAMP(0) ALL_NON_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_NON_LOSSY <Correct error>
TINYBLOB TINYBLOB ALL_LOSSY <Correct value>
TINYBLOB BLOB ALL_LOSSY <Correct error>
TINYBLOB MEDIUMBLOB ALL_LOSSY <Correct error>
Expand Down Expand Up @@ -595,6 +615,8 @@ DATE DATETIME(0) ALL_LOSSY <Correct error>
DATETIME(6) DATE ALL_LOSSY <Correct value>
DATETIME(6) DATE ALL_LOSSY <Correct value>
DATETIME(0) DATE ALL_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_LOSSY <Correct error>
TINYBLOB TINYBLOB ALL_LOSSY,ALL_NON_LOSSY <Correct value>
TINYBLOB BLOB ALL_LOSSY,ALL_NON_LOSSY <Correct value>
TINYBLOB MEDIUMBLOB ALL_LOSSY,ALL_NON_LOSSY <Correct value>
Expand Down Expand Up @@ -759,8 +781,11 @@ DATE DATETIME(0) ALL_LOSSY,ALL_NON_LOSSY <Correct value>
DATETIME(6) DATE ALL_LOSSY,ALL_NON_LOSSY <Correct value>
DATETIME(6) DATE ALL_LOSSY,ALL_NON_LOSSY <Correct value>
DATETIME(0) DATE ALL_LOSSY,ALL_NON_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_LOSSY,ALL_NON_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_LOSSY,ALL_NON_LOSSY <Correct error>
DROP TABLE type_conversions;
call mtr.add_suppression("Slave SQL.*Column 1 of table .test.t1. cannot be converted from type.* error.* 1677");
call mtr.add_suppression("Slave: Got error.*: 1030");
connection master;
DROP TABLE t1;
connection slave;
Expand Down
4 changes: 2 additions & 2 deletions mysql-test/suite/rpl/include/check_type.inc
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


However, variable substitution for SQL commands can’t work with strings containing both ' and ", which affects suite/rpl/include/check_type.inc (and others, such as include/write_var_to_file.inc), so I switched to Last_SQL_Errno instead.
(Help thread: Escaping quotes for --eval in MTR)

Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ if ($can_convert) {
if (!$can_convert) {
connection slave;
wait_for_slave_to_stop;
let $error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Error, 1);
let $error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
eval INSERT INTO type_conversions SET
Source = "$source_type",
Target = "$target_type",
Flags = @@slave_type_conversions,
On_Master = $source_value,
Error = "$error";
Error = $error;
SET GLOBAL SQL_SLAVE_SKIP_COUNTER = 1;
START SLAVE;
}
23 changes: 23 additions & 0 deletions mysql-test/suite/rpl/include/type_conversions.test
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the discussion on Zoom, this should error even with @@slave_type_conversions=ALL_LOSSY enabled (instead of clamping to Y2038), because ‘lossy’ is intended more for “(decimal) precision” than “range maximum”.
I still put the test in @@slave_type_conversions’s script, not only to utilize suite/rpl/include/check_type.inc, but also to assert that @@slave_type_conversions doesn’t change the treatment.

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# File containing different lossy and non-lossy type conversions.
--source include/have_debug.inc

# Integral conversion testing, we do not reduce the test using
# transitivity of conversions since the implementation is not using a
Expand Down Expand Up @@ -1267,6 +1268,28 @@ let $source_temp_format=;
let $target_temp_format=;
--echo # End of MDEV-17098

--echo #
--echo # MDEV-39240 Invalid / MDEV-32188-only TIMESTAMPs
--echo #
--connection master
SET @save_dbug= @@GLOBAL.debug_dbug;
SET @@GLOBAL.debug_dbug= '+d,rpl_pack_simulate_negation';
let $source_type= TIMESTAMP(0);
let $target_type= TIMESTAMP(0);

let $source_value= '0000-00-00 00:00:00'; # ~0 = Y2106 Epochalypse II
let $target_value= FROM_UNIXTIME((1<<31) - 1); # Y2038 Epochalypse I
let $can_convert = 1;
source suite/rpl/include/check_type.inc;

let $source_value= FROM_UNIXTIME(1); # ~1 = not at an Epochalypse
let $can_convert = 0;
source suite/rpl/include/check_type.inc;

--connection master
SET @@GLOBAL.debug_dbug= @save_dbug;
--echo # End of MDEV-39240


--source include/rpl_reset.inc
enable_query_log;
27 changes: 26 additions & 1 deletion mysql-test/suite/rpl/r/rpl_typeconv.result
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ On_Master LONGTEXT,
On_Slave LONGTEXT,
Expected LONGTEXT,
Compare INT,
Error TEXT);
Error INT2);
SELECT @@global.slave_type_conversions;
@@global.slave_type_conversions

Expand Down Expand Up @@ -51,6 +51,10 @@ SET GLOBAL SLAVE_TYPE_CONVERSIONS='';
# MDEV-17098 DATE <-> DATETIME
#
# End of MDEV-17098
#
# MDEV-39240 Invalid / MDEV-32188-only TIMESTAMPs
#
# End of MDEV-39240
include/rpl_reset.inc
connection slave;
SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_NON_LOSSY';
Expand All @@ -67,6 +71,10 @@ SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_NON_LOSSY';
# MDEV-17098 DATE <-> DATETIME
#
# End of MDEV-17098
#
# MDEV-39240 Invalid / MDEV-32188-only TIMESTAMPs
#
# End of MDEV-39240
include/rpl_reset.inc
connection slave;
SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_LOSSY';
Expand All @@ -83,6 +91,10 @@ SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_LOSSY';
# MDEV-17098 DATE <-> DATETIME
#
# End of MDEV-17098
#
# MDEV-39240 Invalid / MDEV-32188-only TIMESTAMPs
#
# End of MDEV-39240
include/rpl_reset.inc
connection slave;
SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_LOSSY,ALL_NON_LOSSY';
Expand All @@ -99,6 +111,10 @@ SET GLOBAL SLAVE_TYPE_CONVERSIONS='ALL_LOSSY,ALL_NON_LOSSY';
# MDEV-17098 DATE <-> DATETIME
#
# End of MDEV-17098
#
# MDEV-39240 Invalid / MDEV-32188-only TIMESTAMPs
#
# End of MDEV-39240
include/rpl_reset.inc
connection slave;
**** Result of conversions ****
Expand Down Expand Up @@ -267,6 +283,8 @@ DATE DATETIME(0) <Correct error>
DATETIME(6) DATE <Correct error>
DATETIME(6) DATE <Correct error>
DATETIME(0) DATE <Correct error>
TIMESTAMP(0) TIMESTAMP(0) <Correct value>
TIMESTAMP(0) TIMESTAMP(0) <Correct error>
TINYBLOB TINYBLOB ALL_NON_LOSSY <Correct value>
TINYBLOB BLOB ALL_NON_LOSSY <Correct value>
TINYBLOB MEDIUMBLOB ALL_NON_LOSSY <Correct value>
Expand Down Expand Up @@ -431,6 +449,8 @@ DATE DATETIME(0) ALL_NON_LOSSY <Correct value>
DATETIME(6) DATE ALL_NON_LOSSY <Correct error>
DATETIME(6) DATE ALL_NON_LOSSY <Correct error>
DATETIME(0) DATE ALL_NON_LOSSY <Correct error>
TIMESTAMP(0) TIMESTAMP(0) ALL_NON_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_NON_LOSSY <Correct error>
TINYBLOB TINYBLOB ALL_LOSSY <Correct value>
TINYBLOB BLOB ALL_LOSSY <Correct error>
TINYBLOB MEDIUMBLOB ALL_LOSSY <Correct error>
Expand Down Expand Up @@ -595,6 +615,8 @@ DATE DATETIME(0) ALL_LOSSY <Correct error>
DATETIME(6) DATE ALL_LOSSY <Correct value>
DATETIME(6) DATE ALL_LOSSY <Correct value>
DATETIME(0) DATE ALL_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_LOSSY <Correct error>
TINYBLOB TINYBLOB ALL_LOSSY,ALL_NON_LOSSY <Correct value>
TINYBLOB BLOB ALL_LOSSY,ALL_NON_LOSSY <Correct value>
TINYBLOB MEDIUMBLOB ALL_LOSSY,ALL_NON_LOSSY <Correct value>
Expand Down Expand Up @@ -759,8 +781,11 @@ DATE DATETIME(0) ALL_LOSSY,ALL_NON_LOSSY <Correct value>
DATETIME(6) DATE ALL_LOSSY,ALL_NON_LOSSY <Correct value>
DATETIME(6) DATE ALL_LOSSY,ALL_NON_LOSSY <Correct value>
DATETIME(0) DATE ALL_LOSSY,ALL_NON_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_LOSSY,ALL_NON_LOSSY <Correct value>
TIMESTAMP(0) TIMESTAMP(0) ALL_LOSSY,ALL_NON_LOSSY <Correct error>
DROP TABLE type_conversions;
call mtr.add_suppression("Slave SQL.*Column 1 of table .test.t1. cannot be converted from type.* error.* 1677");
call mtr.add_suppression("Slave: Got error.*: 1030");
connection master;
DROP TABLE t1;
connection slave;
Expand Down
5 changes: 3 additions & 2 deletions mysql-test/suite/rpl/t/rpl_typeconv.test
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ CREATE TABLE type_conversions (
On_Slave LONGTEXT,
Expected LONGTEXT,
Compare INT,
Error TEXT);
Error INT2);

SELECT @@global.slave_type_conversions;
SET GLOBAL SLAVE_TYPE_CONVERSIONS='';
Expand Down Expand Up @@ -60,7 +60,7 @@ disable_query_log;
SELECT RPAD(Source, 15, ' ') AS Source_Type,
RPAD(Target, 15, ' ') AS Target_Type,
RPAD(Flags, 25, ' ') AS All_Type_Conversion_Flags,
IF(Compare IS NULL AND Error IS NOT NULL, '<Correct error>',
IF(Compare IS NULL AND Error, '<Correct error>',
IF(Compare, '<Correct value>',
CONCAT("'", On_Slave, "' != '", Expected, "'")))
AS Value_On_Slave
Expand All @@ -69,6 +69,7 @@ enable_query_log;
DROP TABLE type_conversions;

call mtr.add_suppression("Slave SQL.*Column 1 of table .test.t1. cannot be converted from type.* error.* 1677");
call mtr.add_suppression("Slave: Got error.*: 1030");

connection master;
DROP TABLE t1;
Expand Down
42 changes: 40 additions & 2 deletions sql/rpl_record.cc
Comment thread
ParadoxV5 marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,15 @@ pack_row(TABLE *table, MY_BITMAP const* cols,
length is stored in little-endian format, since this is the
format used for the binlog.
*/
#if !defined DBUG_OFF && defined DBUG_TRACE
const uchar *old_pack_ptr= pack_ptr;
#ifndef DBUG_OFF
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just cleanup? The patch should be null-merged into 11.8, so let's keep it as minimal as possible. Though you can do the cleanup as a separate patch if you'd like.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s to be careful: old_pack_ptr was previously only used for a DBUG_PRINT, but is now also used for my new DBUG_EXECUTE_IF block.

uchar *old_pack_ptr= pack_ptr;
#endif
pack_ptr= field->pack(pack_ptr, field->ptr + offset,
field->max_data_length());
DBUG_EXECUTE_IF("rpl_pack_simulate_negation",
for (uchar *byte= old_pack_ptr; byte < pack_ptr; ++byte)
*byte= ~*byte;
);
Comment on lines +108 to +111
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered sneaking over-max timestamps into the primary server, but I’m concerned that it’s too UB from reading the relevant entry code, and so decided not to fix any tangential problems this hack surfaces.

DBUG_PRINT("debug", ("field: %s; real_type: %d, pack_ptr: %p;"
" pack_ptr':%p; bytes: %d",
field->field_name.str, field->real_type(),
Expand Down Expand Up @@ -187,6 +191,8 @@ pack_row(TABLE *table, MY_BITMAP const* cols,
A generic, internal, error caused the unpacking to fail.
@retval HA_ERR_CORRUPT_EVENT
Found error when trying to unpack fields.
@retval HA_ERR_ROWS_EVENT_APPLY
Found error when validating field values.
*/
#if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
int
Expand Down Expand Up @@ -338,6 +344,38 @@ unpack_row(rpl_group_info *rgi,
table->s->table_name.str);
DBUG_RETURN(HA_ERR_CORRUPT_EVENT);
}

// Validate this external data
Comment thread
ParadoxV5 marked this conversation as resolved.
switch (f->type()) {
case MYSQL_TYPE_TIMESTAMP:
{
ulong microseconds;
my_time_t seconds= f->get_timestamp(&microseconds);
if (likely(microseconds <= TIME_MAX_SECOND_PART))
{
if (likely(seconds >= 0 && seconds <= TIMESTAMP_MAX_VALUE))
break;
else if (likely(seconds == UINT_MAX32)) // They are both signed.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my_time_t and UINT_MAX32 are both signed before MDEV-32188 (11.5.1).
(-1’s equivalence is only de facto.)

{
// Normalize MariaDB 11.5.1+ Epochalypse
f->store_timestamp(TIMESTAMP_MAX_VALUE, microseconds);
break;
}
}
static const char unixtime_format[]=
"FROM_UNIXTIME(%ld + %lu/1""000""000)";
// + strlen("2147483648""16777215") - strlen("%ld""%lu")
char unixtime[sizeof(unixtime_format) + 12];
snprintf(unixtime, sizeof(unixtime), unixtime_format,
seconds, microseconds);
rgi->rli->report(ERROR_LEVEL, ER_TRUNCATED_WRONG_VALUE_FOR_FIELD,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the commit message where you say "error on the others" it would be good to extend it with the actual error that is thrown.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve reworded along with a base sync (and to rerun CI while here).

rgi->gtid_info(), ER(ER_TRUNCATED_WRONG_VALUE_FOR_FIELD),
f->type_handler()->name().ptr(), unixtime, table->s->db.str,
table->s->table_name.str, f->field_name.str, 0lu);
DBUG_RETURN(HA_ERR_ROWS_EVENT_APPLY);
}
default:;
}
}

/*
Expand Down