[Maria-developers] MDEV-7502 Automatic provisioning of slave - error reporting
Hello Kristian, with general idea working (sending of database / table structures + table data), my next plan is to improve / introduce error handling in existing code along with error handling on slave side (ddl events not from provisioning, ignore errors when updating not yet existing rows, ...). What are general guidelines for introduction of new error codes? Is there anything else I need to do except adding it to /sql/share/errmsg-utf8.txt ? I would create only one new error code - ER_PROVISIONING_FAILED - and place specific messages inside formatted text - same way as ER_MASTER_FATAL_ERROR_READING_BINLOG works. It doesn't go well with translations, but... you can decide. Maybe create different error codes at least for errors, where we are expecting user to fix them - table without index, statement mode. Also, how to tell slave from master that provisioning finished? Is introduction of new error code ER_PROVISIONING_DONE acceptable solution? The error itself would never produce error message in logs. Alternatively I can reuse / create similar event to Stop_log_event, note - provisioning is completed after SQL thread processes whole relay log, not when IO thread receives this event, so even reuse would require additional code. And side question, is there any log event other than Query_log_event which can produce ddl statement? Martin ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] Please review: MDEV-7824 [Bug #68041] Zero date can be inserted in strict no-zero mode through a default value
Hi Sergei, This is a new version. Please also see some comments below. Thanks. On 06/25/2015 01:00 PM, Sergei Golubchik wrote: Hi, Alexander! On Jun 25, Alexander Barkov wrote: Hi Sergei, diff --git a/sql/field_conv.cc b/sql/field_conv.cc index e31f7c5..14f2947 100644 --- a/sql/field_conv.cc +++ b/sql/field_conv.cc @@ -859,7 +859,8 @@ bool memcpy_field_possible(Field *to,Field *from) from-charset() == to-charset() (!sql_mode_for_dates(to-table-in_use) || (from-type()!= MYSQL_TYPE_DATE -from-type()!= MYSQL_TYPE_DATETIME)) +from-type()!= MYSQL_TYPE_DATETIME +from-type()!= MYSQL_TYPE_TIMESTAMP)) what is this for - to apply TIME_NO_ZERO_DATE to timestamps? Right, this is for CREATE TABLE AS SELECT, like in here: set sql_mode=default; drop table if exists t1; create table t1 (a timestamp); insert into t1 values (0); set sql_mode='TRADITIONAL'; drop table if exists t2; create table t2 as select * from t1; The last statement should fail. It does not fail before the patch. This is not strictly DEFAULT. But a very related thing, when a wrong value sneaks in going around Field_xxx::store(). Would you mind if I have this change in this patch? I created a separate MDEV-8373 for CREATE AS SELECT. better not. it's trivial to put it in a separate commit with git citool if it'd be difficult to extract it - then yes, but mention is explicitly in the commit comment (also fixes the case). I am adding tests for both bugs: MDEV-7824 [Bug #68041] Zero date can be inserted in strict no-zero mode through a default value MDEV-8373 Zero date can be inserted in strict no-zero mode through CREATE TABLE AS SELECT timestamp_field into a new shared file mysql-test/include/type_temporal_zero_default.inc Does git citool support partial commit in such case? I.e. if I want to add a part of a new file in the first commit, and then add the rest of the new file in the second commit? diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt index 0846740..d79fbcc 100644 --- a/sql/share/errmsg-utf8.txt +++ b/sql/share/errmsg-utf8.txt @@ -7127,3 +7127,5 @@ ER_ROLE_DROP_EXISTS eng Can't drop role '%-.64s'; it doesn't exist ER_CANNOT_CONVERT_CHARACTER eng Cannot convert '%s' character 0x%-.64s to '%s' +ER_INVALID_DEFAULT_VALUE_FOR_FIELD 22007 +eng Incorrect default value '%-.128s' for column '%.192s' Do you agree with a new error? yes, okay. Regards, Sergei diff --git a/mysql-test/include/type_temporal_zero_default.inc b/mysql-test/include/type_temporal_zero_default.inc new file mode 100644 index 000..500d25e --- /dev/null +++ b/mysql-test/include/type_temporal_zero_default.inc @@ -0,0 +1,75 @@ +--echo # +--echo # MDEV-7824 [Bug #68041] Zero date can be inserted in strict no-zero mode through a default value +--echo # + +# Testing direct INSERT + +SET sql_mode=DEFAULT; +eval CREATE TABLE t1 (a $type DEFAULT $defval); +SET sql_mode=TRADITIONAL; +--error ER_TRUNCATED_WRONG_VALUE +eval INSERT INTO t1 VALUES ($defval); +--error ER_INVALID_DEFAULT_VALUE_FOR_FIELD +INSERT INTO t1 VALUES (); +--error ER_INVALID_DEFAULT_VALUE_FOR_FIELD +INSERT INTO t1 VALUES (DEFAULT); +DROP TABLE t1; +SET sql_mode=DEFAULT; + + +# Testing INSERT .. SELECT + +eval CREATE TABLE t1 (a $type NOT NULL DEFAULT $defval, b $type NOT NULL DEFAULT $defval); +eval CREATE TABLE t2 (a $type NOT NULL DEFAULT $defval); +eval INSERT INTO t2 VALUES ($defval); +SET sql_mode=TRADITIONAL; +--error ER_INVALID_DEFAULT_VALUE_FOR_FIELD +INSERT INTO t1 (a) SELECT a FROM t2; +DROP TABLE t1, t2; +SET sql_mode=DEFAULT; + + +# Testing LOAD + +--eval CREATE TABLE t1 (a $type DEFAULT $defval, b $type DEFAULT $defval) +--eval INSERT INTO t1 VALUES (DEFAULT,DEFAULT); +--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR +--eval SELECT a INTO OUTFILE '$MYSQLTEST_VARDIR/tmp/mdev-7824.txt' FROM t1 +DELETE FROM t1; +SET sql_mode=TRADITIONAL; +--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR +--error ER_INVALID_DEFAULT_VALUE_FOR_FIELD +--eval LOAD DATA INFILE '$MYSQLTEST_VARDIR/tmp/mdev-7824.txt' INTO TABLE t1 (a) +--remove_file $MYSQLTEST_VARDIR/tmp/mdev-7824.txt +DROP TABLE t1; +SET sql_mode=DEFAULT; + +# Testing ALTER when an old field default becomes invalid +# Return an error, even if there is no STRICT_XXX_TABLES set +--eval CREATE TABLE t1 (a $type DEFAULT $defval); +SET sql_mode='NO_ZERO_DATE'; +--error ER_INVALID_DEFAULT +ALTER TABLE t1 ADD b INT NOT NULL; +DROP TABLE t1; +SET sql_mode=DEFAULT; + + +--echo # +--echo # End of MDEV-7824 [Bug #68041] Zero date can be inserted in strict no-zero mode through a default value +--echo # + +--echo # +--echo # MDEV-8373 Zero date can be inserted in strict no-zero mode through CREATE TABLE AS SELECT timestamp_field +--echo # + +SET sql_mode=DEFAULT; +--eval CREATE TABLE t1 (a $type); +INSERT INTO t1 VALUES (0); +SET sql_mode='TRADITIONAL'; +--error ER_TRUNCATED_WRONG_VALUE
Re: [Maria-developers] Please review: MDEV-7824 [Bug #68041] Zero date can be inserted in strict no-zero mode through a default value
Hi Sergei,
Thanks for review.
I have a couple of questions before I can send a new version.
See the questions and other comments below.
On 06/24/2015 08:32 PM, Sergei Golubchik wrote:
Hi, Alexander!
I rewrote the patch slightly, so now we don't need to remember
all_default_are_checked or write_set_defaults_are_checked.
The default values are now checked before the query execution
and before the first restore_record() call. They are now
checked directly in table-s-default_values.
I hope it's now faster and clearer comparing to the original patch
version from MySQL-5.6.
Agree! It's much better now.
See my comments and questions below.
diff --git a/sql/field.h b/sql/field.h
index 4ca493e..2b430cb 100644
--- a/sql/field.h
+++ b/sql/field.h
This is ok, but then, please, see what other places in the code you can
change to use your new helpers. In a separate commit, of course. I
suspect there's a lot of code that's doing, like
/* Get the value from default_values */
diff= (my_ptrdiff_t) (orig_field-table-s-default_values-
orig_field-table-record[0]);
orig_field-move_field_offset(diff); // Points now at default_values
if (orig_field-is_real_null())
field-set_null();
else
{
field-set_notnull();
memcpy(field-ptr, orig_field-ptr, field-pack_length());
}
orig_field-move_field_offset(-diff); // Back to record[0]
(this is from create_tmp_table() in sql_select.cc)
Sure, I have created a task for this, not to forget:
MDEV-8372 Use helper methods introduced in MDEV-7824 all around the code
diff --git a/sql/field.cc b/sql/field.cc
index ba6d4ff..4a854a3 100644
--- a/sql/field.cc
+++ b/sql/field.cc
@@ -5980,12 +5993,22 @@ String *Field_newdate::val_str(String *val_buffer,
bool Field_newdate::get_date(MYSQL_TIME *ltime,ulonglong fuzzydate)
{
uint32 tmp=(uint32) uint3korr(ptr);
- ltime-day= tmp 31;
- ltime-month= (tmp 5) 15;
+ ltime-day= num_to_day(tmp);
+ ltime-month= num_to_month(tmp);
ltime-year= (tmp 9);
may be num_to_year() ? just for consistency.
ltime-time_type= MYSQL_TIMESTAMP_DATE;
ltime-hour= ltime-minute= ltime-second= ltime-second_part= ltime-neg=
0;
- return validate_for_get_date(tmp, ltime, fuzzydate);
+ return validate_MMDD(tmp, ltime-month, ltime-day, fuzzydate);
+}
+
+
+bool
+Field_newdate::validate_value_in_record(THD *thd, const uchar *record) const
+{
+ DBUG_ASSERT(!is_null_in_record(record));
+ uint32 num= (uint32) uint3korr(ptr_in_record(record));
+ return validate_MMDD(num, num_to_month(num), num_to_day(num),
on the other hand, I'd rather use get_date() here, instead of
duplicating its logic. This applies to all other
validate_value_in_record() methods too. May be like
MYSQL_TIME ltime;
get_date_from_ptr(ltime, ptr_in_record(record), sql_mode_for_dates(thd));
get_date_from_ptr() will generally need to do more than
it's needed for validation. For example, for DATETIME, it will spend
CPU to initialize the TIME part unnecessarily.
But Field_datetimef does full initialization of MYSQL_TIME anyway,
as I didn't add a faster validation function in compat56.cc.
Okey, as validation is done only once per query, I'll agree that
it's better to have less duplicate code here than a super-optimized code.
I'll try your idea with get_date_from_ptr().
+ sql_mode_for_dates(thd));
}
@@ -10268,3 +10332,29 @@ void Field::set_explicit_default(Item *value)
return;
set_has_explicit_value();
}
+
+
+bool Field::validate_value_in_record_with_warn(THD *thd, const uchar *record)
+{
+ if (!validate_value_in_record(thd, record))
+return false;
+ /*
+Only write_set[field_index] is set for this,
+while read_set[field_index] is not guaranteed to be set.
+Set table-read_set to NULL,
+to make ASSERT_COLUMN_MARKED_FOR_READ in val_str() happy.
+ */
+ MY_BITMAP *read_set_backup= table-read_set;
+ table-read_set= 0;
Normally one uses dbug_tmp_use_all_columns() for that (without comment).
Thanks. Will use this.
+ // Get val_str() for the DEFAULT value
+ StringBufferMAX_FIELD_WIDTH tmp;
+ val_str(tmp, ptr_in_record(record));
+ // Convert val_str() result to a printable error parameter
+ ErrConvString err(tmp);
+ push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
+ ER_INVALID_DEFAULT_VALUE_FOR_FIELD,
+ ER(ER_INVALID_DEFAULT_VALUE_FOR_FIELD),
+ err.ptr(), field_name);
+ table-read_set= read_set_backup;
+ return true;
+}
diff --git a/sql/field_conv.cc b/sql/field_conv.cc
index e31f7c5..14f2947 100644
--- a/sql/field_conv.cc
+++ b/sql/field_conv.cc
@@ -859,7 +859,8 @@ bool memcpy_field_possible(Field *to,Field *from)
from-charset() == to-charset()
(!sql_mode_for_dates(to-table-in_use) ||
(from-type()!= MYSQL_TYPE_DATE
-from-type()!=
