maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Oleksandr!

On Mar 18, Oleksandr Byelkin wrote:
> revision-id: 21a15ea673c (mariadb-10.2.22-72-g21a15ea673c)
> parent(s): bb8c82c66ab
> author: Oleksandr Byelkin <sanja@xxxxxxxxxxx>
> committer: Oleksandr Byelkin <sanja@xxxxxxxxxxx>
> timestamp: 2019-03-14 15:12:47 +0100
> message:
> 
> MDEV-16240: Assertion `0' failed in row_sel_convert_mysql_key_to_innobase
> 
> Set table in row ID position mode before using this function.
> 
> diff --git a/mysql-test/r/multi_update_innodb.result b/mysql-test/r/multi_update_innodb.result
> index 5890fd24f5f..294ebfcebdf 100644
> --- a/mysql-test/r/multi_update_innodb.result
> +++ b/mysql-test/r/multi_update_innodb.result
> @@ -151,3 +151,43 @@ create table t2 like t1;
>  insert into t2 select * from t1;
>  delete t1,t2 from t2,t1 where t1.a<'B' and t2.b=t1.b;
>  drop table t1,t2;
> +#
> +# MDEV-16240: Assertion `0' failed in
> +# row_sel_convert_mysql_key_to_innobase
> +#
> +SET @save_sql_mode=@@sql_mode;
> +set sql_mode='';
> +CREATE TABLE `t3` (
> +`f1` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE current_timestamp(),
> +`f2` datetime DEFAULT '2000-01-01 00:00:00' ON UPDATE current_timestamp(),
> +`f3` TIMESTAMP NULL DEFAULT '2000-01-01 00:00:00',
> +`pk` datetime NOT NULL DEFAULT '0000-00-00 00:00:00',
> +`f4` datetime DEFAULT current_timestamp(),
> +PRIMARY KEY (`pk`),
> +UNIQUE KEY `f2k` (`f2`),
> +KEY `f4k` (`f4`)
> +) ENGINE=InnoDB;
> +INSERT INTO `t3` VALUES ('2018-05-18 15:08:07','2018-05-18 17:08:07','0000-00-00 00:00:00','0000-00-00 00:00:00','0000-00-00 00:00:00'),('0000-00-00 00:00:00','0000-00-00 00:00:00','1999-12-31 23:00:00','2002-07-03 23:04:40','0000-00-00 00:00:00');
> +CREATE VIEW `v1` AS
> +SELECT `t3`.`pk` AS `pk`,
> +`t3`.`f3` AS `f3`,
> +`t3`.`f4` AS `f4`,
> +`t3`.`f2` AS `f2`,
> +`t3`.`f1` AS `f1`
> +FROM `t3`;
> +CREATE TABLE `t4` (
> +`f1` datetime DEFAULT current_timestamp() ON UPDATE current_timestamp(),
> +`f3` timestamp NULL DEFAULT NULL,
> +`f2` timestamp NULL DEFAULT '1999-12-31 23:00:00' ON UPDATE current_timestamp(),
> +`pk` int(11) NOT NULL,
> +`f4` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
> +PRIMARY KEY (`pk`)
> +) ENGINE=InnoDB;
> +INSERT INTO `t4` VALUES ('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,1,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,3,'2018-05-18 15:08:06'),('0000-00-00 00:00:00','0000-00-00 00:00:00',NULL,1976,'0000-00-00 00:00:00'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2000,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2001,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2002,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2003,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2004,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2005,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2018,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2019,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2024,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','1999-12-31 23:00:00',2025,'2018-05-18 15:08:06'),('0000-00-00 00:00:00',NULL,'2018-05-18 15:08:06',2026,'2018-05-18 15:08:06'),('2018-05-18 17:08:07','0000-00-00 00:00:00','0000-00-00 00:00:00',2027,'0000-00-00 00:00:00');
> +UPDATE `v1` t1, `t4` t2
> +SET t1.`f2` = 6452736 WHERE t1.`f4` = 6272000;
> +ERROR 23000: Duplicate entry '0000-00-00 00:00:00' for key 'f2k'

actually, when I wrote "please, rename all columns in the test to have
unique names" I meant _all_ columns, that is, in the t3 columnd could've
been f1-f5, in v1 they could be called f6-f10, in t4 - f11-f15.

but don't bother renaming them now, apparently the most important name was
'f2k' and it's unique now, so it's sufficiently clear, thanks.

Still, for the future, it's generally helpful to have all names
different to be able to understand what the error message refers to.

> +DROP VIEW v1;
> +DROP TABLE t3,t4;
> +SET @@sql_mode=@save_sql_mode;
> +# End of 10.2 tests
> diff --git a/sql/handler.cc b/sql/handler.cc
> index ae63640ae5c..d307433cf5d 100644
> --- a/sql/handler.cc
> +++ b/sql/handler.cc
> @@ -2648,8 +2648,7 @@ int handler::ha_rnd_pos(uchar *buf, uchar *pos)
>    DBUG_ENTER("handler::ha_rnd_pos");
>    DBUG_ASSERT(table_share->tmp_table != NO_TMP_TABLE ||
>                m_lock_type != F_UNLCK);
> -  /* TODO: Find out how to solve ha_rnd_pos when finding duplicate update. */
> -  /* DBUG_ASSERT(inited == RND); */
> +  DBUG_ASSERT(inited == RND);
>  
>    TABLE_IO_WAIT(tracker, m_psi, PSI_TABLE_FETCH_ROW, MAX_KEY, 0,
>      { result= rnd_pos(buf, pos); })
> @@ -3307,6 +3306,10 @@ void handler::get_auto_increment(ulonglong offset, ulonglong increment,
>    ulonglong nr;
>    int error;
>    MY_BITMAP *old_read_set;
> +  bool rnd_inited= (inited ==  RND);
> +

when is that true?

> diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc
> index 8a9dd083911..cc1649b3c92 100644
> --- a/sql/item_subselect.cc
> +++ b/sql/item_subselect.cc
> @@ -5855,12 +5855,17 @@ Ordered_key::cmp_keys_by_row_data_and_rownum(Ordered_key *key,
>  }
>  
>  
> -void Ordered_key::sort_keys()
> +bool Ordered_key::sort_keys()
>  {
> +  /* Prepare table for random positioning (importent for innodb) */

1) typo: "important" but 2) I don't think you need a comment here at all
now.  There's an assert in ha_rnd_pos(), it's rather more clear and more
reliable anyway.

> +  if (tbl->file->ha_rnd_init(0))
> +    return TRUE;
>    my_qsort2(key_buff, (size_t) key_buff_elements, sizeof(rownum_t),
>              (qsort2_cmp) &cmp_keys_by_row_data_and_rownum, (void*) this);
>    /* Invalidate the current row position. */
>    cur_key_idx= HA_POS_ERROR;
> +  tbl->file->ha_rnd_end();
> +  return FALSE;
>  }
>  
>  
> diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
> index 307473cecbd..5593580dbca 100644
> --- a/sql/sql_insert.cc
> +++ b/sql/sql_insert.cc
> @@ -3177,6 +3193,10 @@ bool Delayed_insert::handle_inserts(void)
>      max_rows= ULONG_MAX;                     // Do as much as possible
>    }
>  
> +  /* Prepare table for random positioning (importent for innodb) */

same

> +  if (table->file->ha_rnd_init(0))
> +    goto err;

what's that for? duplicate key error message?

> +
>    /*
>      We can't use row caching when using the binary log because if
>      we get a crash, then binary log will contain rows that are not yet
> @@ -3649,7 +3671,12 @@ select_insert::prepare(List<Item> &values, SELECT_LEX_UNIT *u)
>  
>    thd->cuted_fields=0;
>    if (info.ignore || info.handle_duplicates != DUP_ERROR)
> +  {
>      table->file->extra(HA_EXTRA_IGNORE_DUP_KEY);
> +    if (table->file->ha_table_flags() & HA_DUPLICATE_POS &&
> +        table->file->ha_rnd_init(0))
> +      DBUG_RETURN(1);

here and in other places, you don't do my_error().
Is there some catch-all my_error somewhere up the stack or it'll be the
usual assert in Diagnostic_area?

> +  }
>    if (info.handle_duplicates == DUP_REPLACE &&
>        (!table->triggers || !table->triggers->has_delete_triggers()))
>      table->file->extra(HA_EXTRA_WRITE_CAN_REPLACE);
> @@ -3929,6 +3959,11 @@ void select_insert::abort_result_set() {
>      if (thd->locked_tables_mode <= LTM_LOCK_TABLES)
>        table->file->ha_end_bulk_insert();
>  
> +    if (table->file->inited)

why not "if (table->file->ha_table_flags() & HA_DUPLICATE_POS)" ?

> +      table->file->ha_rnd_end();
> +    table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
> +    table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE);
> +
>      /*
>        If at least one row has been inserted/modified and will stay in
>        the table (the table doesn't have transactions) we must write to
> diff --git a/sql/sql_load.cc b/sql/sql_load.cc
> index 8c2f17dac3f..3283b962f65 100644
> --- a/sql/sql_load.cc
> +++ b/sql/sql_load.cc
> @@ -638,6 +638,13 @@ int mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
>  
>      thd->abort_on_warning= !ignore && thd->is_strict_mode();
>  
> +    /* Prepare table for random positioning (importent for innodb) */

same

> +    if ((table_list->table->file->ha_table_flags() & HA_DUPLICATE_POS) &&
> +        table_list->table->file->ha_rnd_init(0))
> +    {
> +      error= 1;
> +      goto err;
> +    }
>      thd_progress_init(thd, 2);
>      if (table_list->table->validate_default_values_of_unset_fields(thd))
>      {
> diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> index 91a6445c870..4c401ea9011 100644
> --- a/sql/sql_select.cc
> +++ b/sql/sql_select.cc
> @@ -20317,6 +20317,12 @@ end_unique_update(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
>        table->file->print_error(error,MYF(0));	/* purecov: inspected */
>        DBUG_RETURN(NESTED_LOOP_ERROR);            /* purecov: inspected */
>      }
> +    /* Prepare table for random positioning */
> +    bool rnd_inited= (table->file->inited == handler::RND);

when is that true? and when false?

> +    if (!rnd_inited &&
> +        ((error= table->file->ha_index_end()) ||
> +         (error= table->file->ha_rnd_init(0))))
> +      DBUG_RETURN(NESTED_LOOP_ERROR);
>      if (table->file->ha_rnd_pos(table->record[1],table->file->dup_ref))
>      {
>        table->file->print_error(error,MYF(0));	/* purecov: inspected */
> diff --git a/sql/sql_update.cc b/sql/sql_update.cc
> index 11ffa684216..6d4c11d494a 100644
> --- a/sql/sql_update.cc
> +++ b/sql/sql_update.cc
> @@ -211,6 +211,10 @@ static void prepare_record_for_error_message(int error, TABLE *table)
>    bitmap_union(table->read_set, &unique_map);
>    /* Tell the engine about the new set. */
>    table->file->column_bitmaps_signal();
> +  /* Prepare table for random positioning (importent for innodb) */

same

> +  if (table->file->ha_index_or_rnd_end() ||
> +      table->file->ha_rnd_init(0))
> +    DBUG_VOID_RETURN;
>    /* Read record that is identified by table->file->ref. */
>    (void) table->file->ha_rnd_pos(table->record[1], table->file->ref);
>    /* Copy the newly read columns into the new record. */
> 
Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx