maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Varun,

First, Some input on the test:

On Wed, Mar 27, 2019 at 01:55:16PM +0530, Varun wrote:
> revision-id: e0fa1993692a3552feddae56cbee3078d4da2121 (mariadb-10.2.22-91-ge0fa1993692)
> parent(s): 50a8fc52988d13a5164a1a542b9d7a85e3ecc1c1
> author: Varun Gupta
> committer: Varun Gupta
> timestamp: 2019-03-27 13:48:44 +0530
> message:
> 
> MDEV-18899: Server crashes in Field::set_warning_truncated_wrong_value
> 
> To fix the crash there we need to make sure that the
> server while storing the statistical values in statistical tables should do it
> in a multi-byte safe way.
> Also there is no need to throw warnings if there is truncation while storing
> values from statistical fields.
> 
...
> --- a/mysql-test/r/stat_tables_innodb.result
> +++ b/mysql-test/r/stat_tables_innodb.result
> @@ -651,6 +651,63 @@ SELECT MAX(pk) FROM t1;
>  MAX(pk)
>  NULL
>  DROP TABLE t1;
> +#
> +# MDEV-18899: Server crashes in Field::set_warning_truncated_wrong_value
> +#
> +set names utf8;
> +set @save_optimizer_use_condition_selectivity=@@optimizer_use_condition_selectivity;
> +set optimizer_use_condition_selectivity=4;
> +set use_stat_tables=preferably;
> +set @save_histogram_size= @@histogram_size;
> +set histogram_size=255;
> +create table t1 ( a varchar(255) character set utf8);
> +insert into t1 values (REPEAT('ӥ',255)), (REPEAT('ç',255));
> +analyze table t1;
> +Table	Op	Msg_type	Msg_text
> +test.t1	analyze	status	Engine-independent statistics collected
> +test.t1	analyze	status	OK
> +select HEX(RIGHT(min_value, 1)), length(min_value) from mysql.column_stats;
> +HEX(RIGHT(min_value, 1))	length(min_value)
> +A7	254
> +select HEX(RIGHT(max_value, 1)), length(max_value) from mysql.column_stats;
> +HEX(RIGHT(max_value, 1))	length(max_value)
> +A5	254
> +analyze select * from t1 where a  >= 'ӥ';
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	r_rows	filtered	r_filtered	Extra
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	2	2.00	50.00	50.00	Using where
> +set @save_sql_mode= @@sql_mode;
> +set sql_mode='ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION';
> +update mysql.column_stats set min_value= REPEAT('ӥ',255);

This looks scary, what if there were some other data in these tables? 
Can you change add a WHERE db_name=database() and table_name='t1' ? 

> +Warnings:
> +Warning	1265	Data truncated for column 'min_value' at row 1
> +select HEX(RIGHT(min_value, 1)), length(min_value) from mysql.column_stats;
> +HEX(RIGHT(min_value, 1))	length(min_value)
> +D3	255
> +analyze select * from t1 where a  >= 'ӥ';
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	r_rows	filtered	r_filtered	Extra
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	2	2.00	50.00	50.00	Using where
> +set names latin1;
> +drop table t1;
> +CREATE TABLE t1 (col1 date);
> +INSERT INTO t1 VALUES('2004-01-01'),('2004-02-29');
> +INSERT INTO t1 VALUES('0000-10-31');
> +analyze table t1;
> +Table	Op	Msg_type	Msg_text
> +test.t1	analyze	status	Engine-independent statistics collected
> +test.t1	analyze	status	OK
> +update mysql.column_stats set min_value='2004-0-31123';

The same as above.

> +select min_value from mysql.column_stats;
> +min_value
> +2004-0-31123

This case also makes one wonder whether not producing a warning was a good idea
after all.

A varchar string that is 255-byte long, and after 255 bytes it gets truncated
in the middle of a character is a rare case.

An incorrect datetime value... not as rare?

> +select * from t1;

Is the above supposed to read the EITS data? It doesn't as the query doesn't
use the WHERE clause.
> +col1
> +2004-01-01
> +2004-02-29
> +0000-10-31
> +drop table t1;
> +set @@sql_mode= @save_sql_mode;
>  set use_stat_tables=@save_use_stat_tables;
> +set @@histogram_size= @save_histogram_size;
> +set @@optimizer_use_condition_selectivity=@save_optimizer_use_condition_selectivity;
>  set optimizer_switch=@save_optimizer_switch_for_stat_tables_test;
>  SET SESSION STORAGE_ENGINE=DEFAULT;
> diff --git a/mysql-test/t/stat_tables.test b/mysql-test/t/stat_tables.test
> index b89ab2bbd2d..114c5c97a7e 100644
> --- a/mysql-test/t/stat_tables.test
> +++ b/mysql-test/t/stat_tables.test
> @@ -401,4 +401,44 @@ SELECT MAX(pk) FROM t1;
>  
>  DROP TABLE t1;
>  
> +--echo #
> +--echo # MDEV-18899: Server crashes in Field::set_warning_truncated_wrong_value
> +--echo #
> +
> +set names utf8;
> +set @save_optimizer_use_condition_selectivity=@@optimizer_use_condition_selectivity;
> +set optimizer_use_condition_selectivity=4;
> +set use_stat_tables=preferably;
> +set @save_histogram_size= @@histogram_size;
> +set histogram_size=255;
> +
> +create table t1 ( a varchar(255) character set utf8);
> +insert into t1 values (REPEAT('ӥ',255)), (REPEAT('ç',255));
> +
> +analyze table t1;
> +select HEX(RIGHT(min_value, 1)), length(min_value) from mysql.column_stats;
> +select HEX(RIGHT(max_value, 1)), length(max_value) from mysql.column_stats;
> +analyze select * from t1 where a  >= 'ӥ';
> +
> +set @save_sql_mode= @@sql_mode;
> +set sql_mode='ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION';
> +update mysql.column_stats set min_value= REPEAT('ӥ',255);
> +select HEX(RIGHT(min_value, 1)), length(min_value) from mysql.column_stats;
> +analyze select * from t1 where a  >= 'ӥ';
> +
> +set names latin1;
> +drop table t1;
> +
> +CREATE TABLE t1 (col1 date);
> +INSERT INTO t1 VALUES('2004-01-01'),('2004-02-29');
> +INSERT INTO t1 VALUES('0000-10-31');
> +analyze table t1;
> +update mysql.column_stats set min_value='2004-0-31123';
> +select min_value from mysql.column_stats;
> +select * from t1;
> +drop table t1;
> +
> +set @@sql_mode= @save_sql_mode;
>  set use_stat_tables=@save_use_stat_tables;
> +set @@histogram_size= @save_histogram_size;
> +set @@optimizer_use_condition_selectivity=@save_optimizer_use_condition_selectivity;
> diff --git a/sql/field.cc b/sql/field.cc
> index 080cf34c76d..c26195a3650 100644
> --- a/sql/field.cc
> +++ b/sql/field.cc
> @@ -2219,7 +2219,6 @@ bool Field_str::can_be_substituted_to_equal_item(const Context &ctx,
>    return false;
>  }
>  
> -
>  void Field_num::make_field(Send_field *field)
>  {
>    Field::make_field(field);
> @@ -7023,12 +7022,16 @@ Field_longstr::check_string_copy_error(const String_copier *copier,
>  {
>    const char *pos;
>    char tmp[32];
> +  THD *thd= get_thd();
>  
>    if (!(pos= copier->most_important_error_pos()))
>      return FALSE;
>  
> -  convert_to_printable(tmp, sizeof(tmp), pos, (end - pos), cs, 6);
> -  set_warning_truncated_wrong_value("string", tmp);
> +  if (thd->count_cuted_fields)
> +  {
> +    convert_to_printable(tmp, sizeof(tmp), pos, (end - pos), cs, 6);
> +    set_warning_truncated_wrong_value("string", tmp);
> +  }
>    return TRUE;
>  }
>  
> diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc
> index b5811c683e8..0960a9a1ec5 100644
> --- a/sql/sql_statistics.cc
> +++ b/sql/sql_statistics.cc
> @@ -1044,6 +1044,7 @@ class Column_stat: public Stat_table
>    {
>      char buff[MAX_FIELD_WIDTH];
>      String val(buff, sizeof(buff), &my_charset_bin);
> +    uint32 length= 0;
>  
>      for (uint i= COLUMN_STAT_MIN_VALUE; i <= COLUMN_STAT_HISTOGRAM; i++)
>      {  
> @@ -1060,7 +1061,9 @@ class Column_stat: public Stat_table
>            else
>            {
>              table_field->collected_stats->min_value->val_str(&val);
> -            stat_field->store(val.ptr(), val.length(), &my_charset_bin);
> +            length= Well_formed_prefix(val.charset(), val.ptr(),
> +                           MY_MIN(val.length(), stat_field->field_length)).length();
> +            stat_field->store(val.ptr(), length, &my_charset_bin);
>            }
>            break;
>          case COLUMN_STAT_MAX_VALUE:
> @@ -1069,7 +1072,9 @@ class Column_stat: public Stat_table
>            else
>            {
>              table_field->collected_stats->max_value->val_str(&val);
> -            stat_field->store(val.ptr(), val.length(), &my_charset_bin);
> +            length= Well_formed_prefix(val.charset(), val.ptr(),
> +                            MY_MIN(val.length(), stat_field->field_length)).length();
> +            stat_field->store(val.ptr(), length, &my_charset_bin);
>            }
>            break;
>          case COLUMN_STAT_NULLS_RATIO:
> @@ -2934,7 +2939,6 @@ int update_statistics_for_table(THD *thd, TABLE *table)
>    DBUG_RETURN(rc);
>  }
>  
> -
>  /**
>    @brief
>    Read statistics for a table from the persistent statistical tables
> @@ -2980,6 +2984,7 @@ int read_statistics_for_table(THD *thd, TABLE *table, TABLE_LIST *stat_tables)
>    Table_statistics *read_stats= table_share->stats_cb.table_stats;
>  
>    DBUG_ENTER("read_statistics_for_table");
> +  DBUG_ASSERT(thd->count_cuted_fields == CHECK_FIELD_IGNORE);
>  
>    /* Read statistics from the statistical table table_stats */
>    stat_table= stat_tables[TABLE_STAT].table;
> @@ -3059,7 +3064,7 @@ int read_statistics_for_table(THD *thd, TABLE *table, TABLE_LIST *stat_tables)
>        }
>      }
>    }
> -      
> +
>    table->stats_is_read= TRUE;
>  
>    DBUG_RETURN(0);

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog