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
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 assume we only need to cover Row-based Replication here, since changing Statement-based Replication requires a mode (be it @@sql_mode or replication-exclusive) that normalizes Y2106 in multiple sources of timestamp values.

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
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
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.

We could use rli->relay_log.description_event_for_exec->server_version_split (which rpl_master_has_bug() uses) to decide what to do with the new unsigned range, but I believe they need to be validated for any primary’s version like any external input.

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,
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