maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander!

On Apr 26, Alexander Barkov wrote:
> revision-id: 8a990ad1774
> author: Alexander Barkov <bar@xxxxxxxxxxx>
> committer: Alexander Barkov <bar@xxxxxxxxxxx>
> timestamp: 2019-03-25 19:19:48 +0400
> message: MDEV-18319 BIGINT UNSIGNED Performance issue
> 
> diff --git a/mysql-test/main/errors.result b/mysql-test/main/errors.result
> index ba05a2b37d45..96ad96395aab 100644
> --- a/mysql-test/main/errors.result
> +++ b/mysql-test/main/errors.result
> @@ -32,16 +32,21 @@ set sql_mode=default;
>  CREATE TABLE t1 (a INT);
>  SELECT a FROM t1 WHERE a IN(1, (SELECT IF(1=0,1,2/0)));
>  a
> +Warnings:
> +Warning	1365	Division by 0

Interesting. There was no warning before.

>  INSERT INTO t1 VALUES(1);
>  SELECT a FROM t1 WHERE a IN(1, (SELECT IF(1=0,1,2/0)));
>  a
>  1
> diff --git a/mysql-test/main/func_in.result b/mysql-test/main/func_in.result
> index 65313148bf80..59822ae1049c 100644
> --- a/mysql-test/main/func_in.result
> +++ b/mysql-test/main/func_in.result
> @@ -489,6 +489,7 @@ SELECT id FROM t1 WHERE id IN(4564, (SELECT IF(1=0,1,1/0)) );
>  id
>  Warnings:
>  Warning	1365	Division by 0
> +Warning	1365	Division by 0

This is not very nice. May be we should use Item_cache in these cases?

>  DROP TABLE t1;
>  End of 5.0 tests
>  create table t1(f1 char(1));
> @@ -909,3 +910,71 @@ Warnings:
>  Warning	1292	Truncated incorrect time value: ''
>  Warning	1292	Truncated incorrect time value: ''
>  Warning	1292	Truncated incorrect time value: ''
> +#
> +# MDEV-18319 BIGINT UNSIGNED Performance issue
> +#
> +CREATE TABLE t1 (
> +id bigint(20) unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY
> +);
> +FOR i IN 0..255
> +DO
> +INSERT INTO t1 VALUES ();
> +END FOR
> +$$
> +SELECT MIN(id), MAX(id), COUNT(*) FROM t1;
> +MIN(id)	MAX(id)	COUNT(*)
> +1	256	256

interesting. why it's min=1 and max=255, if your loop is in 0..255?

> +EXPLAIN SELECT id FROM t1 WHERE id IN (1,2);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +EXPLAIN SELECT id FROM t1 WHERE id IN (9223372036854775806, 9223372036854775807);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +EXPLAIN SELECT id FROM t1 WHERE id IN (9223372036854775807, 9223372036854775808);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +EXPLAIN SELECT id FROM t1 WHERE id IN (1.0,2.0);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +EXPLAIN SELECT id FROM t1 WHERE id IN (9223372036854775806.0, 9223372036854775807.0);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +# Cannot compare this as INT (yet)
> +EXPLAIN SELECT id FROM t1 WHERE id IN (9223372036854775807.0, 9223372036854775808.0);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	index	PRIMARY	PRIMARY	8	NULL	256	Using where; Using index
> +DROP TABLE t1;
> +CREATE TABLE t1 (
> +id bigint(20) NOT NULL AUTO_INCREMENT PRIMARY KEY
> +);
> +FOR i IN 0..255
> +DO
> +INSERT INTO t1 VALUES ();
> +END FOR
> +$$
> +SELECT MIN(id), MAX(id), COUNT(*) FROM t1;
> +MIN(id)	MAX(id)	COUNT(*)
> +1	256	256
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-1,-2);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775806, -9223372036854775807);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775807, -9223372036854775808);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-1.0,-2.0);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775806.0, -9223372036854775807.0);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775807.0, -9223372036854775808.0);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	PRIMARY	PRIMARY	8	NULL	2	Using where; Using index
> +# Cannot compare this as INT (yet)
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775808.0, -9223372036854775809.0);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	index	PRIMARY	PRIMARY	8	NULL	256	Using where; Using index
> +DROP TABLE t1;
> diff --git a/mysql-test/main/func_in.test b/mysql-test/main/func_in.test
> index b99fad159c22..04b0b328c27b 100644
> --- a/mysql-test/main/func_in.test
> +++ b/mysql-test/main/func_in.test
> @@ -690,3 +690,51 @@ SELECT
>    TIME'00:00:00'='' AS c1_true,
>    TIME'00:00:00' IN ('', TIME'10:20:30') AS c2_true,
>    TIME'00:00:00' NOT IN ('', TIME'10:20:30') AS c3_false;
> +
> +--echo #
> +--echo # MDEV-18319 BIGINT UNSIGNED Performance issue
> +--echo #
> +
> +CREATE TABLE t1 (
> +  id bigint(20) unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY
> +);
> +DELIMITER $$;
> +FOR i IN 0..255
> +DO
> +  INSERT INTO t1 VALUES ();
> +END FOR
> +$$
> +DELIMITER ;$$

A couple of one-liners instead:

  insert t1 select seq from seq_1_to_256;

or, in pure SQL

  insert t1 with recursive g(i) as (select 1 union select i+1 from g where i < 257) select * from g;

> +SELECT MIN(id), MAX(id), COUNT(*) FROM t1;
> +EXPLAIN SELECT id FROM t1 WHERE id IN (1,2);
> +EXPLAIN SELECT id FROM t1 WHERE id IN (9223372036854775806, 9223372036854775807);
> +EXPLAIN SELECT id FROM t1 WHERE id IN (9223372036854775807, 9223372036854775808);
> +
> +EXPLAIN SELECT id FROM t1 WHERE id IN (1.0,2.0);
> +EXPLAIN SELECT id FROM t1 WHERE id IN (9223372036854775806.0, 9223372036854775807.0);
> +--echo # Cannot compare this as INT (yet)
> +EXPLAIN SELECT id FROM t1 WHERE id IN (9223372036854775807.0, 9223372036854775808.0);
> +DROP TABLE t1;
> +
> +
> +CREATE TABLE t1 (
> +  id bigint(20) NOT NULL AUTO_INCREMENT PRIMARY KEY
> +);
> +DELIMITER $$;
> +FOR i IN 0..255
> +DO
> +  INSERT INTO t1 VALUES ();
> +END FOR
> +$$
> +DELIMITER ;$$
> +SELECT MIN(id), MAX(id), COUNT(*) FROM t1;
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-1,-2);
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775806, -9223372036854775807);
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775807, -9223372036854775808);
> +
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-1.0,-2.0);
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775806.0, -9223372036854775807.0);
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775807.0, -9223372036854775808.0);
> +--echo # Cannot compare this as INT (yet)
> +EXPLAIN SELECT id FROM t1 WHERE id IN (-9223372036854775808.0, -9223372036854775809.0);
> +DROP TABLE t1;
> diff --git a/sql/item.h b/sql/item.h
> index 883cc791f384..9f215c74864b 100644
> --- a/sql/item.h
> +++ b/sql/item.h
> @@ -70,6 +70,11 @@ class Value: public st_value
>    bool is_temporal() const { return m_type == DYN_COL_DATETIME; }
>    bool is_string() const { return m_type == DYN_COL_STRING; }
>    bool is_decimal() const { return m_type == DYN_COL_DECIMAL; }
> +  Longlong_hybrid to_longlong_hybrid_native() const
> +  {
> +    DBUG_ASSERT(is_longlong());
> +    return Longlong_hybrid(value.m_longlong, m_type == DYN_COL_UINT);
> +  }
>  };
>  
>  
> diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
> index b2fa753f2bd2..c7176df43d6a 100644
> --- a/sql/item_cmpfunc.cc
> +++ b/sql/item_cmpfunc.cc
> @@ -3747,11 +3748,45 @@ bool Predicant_to_list_comparator::add_value(const char *funcname,
>    Item *tmpargs[2];
>    tmpargs[0]= args->arguments()[m_predicant_index];
>    tmpargs[1]= args->arguments()[value_index];
> -  if (tmp.aggregate_for_comparison(funcname, tmpargs, 2, true))
> +  if (tmp.aggregate_for_comparison(funcname, tmpargs, 2, false))

How would that work? Are you going to compare signed to unsigned as
longlongs?

>    {
>      DBUG_ASSERT(current_thd->is_error());
>      return true;
>    }
> +  /*
> +    Try to compare using type handler of the predicant when possible,
> +    as this can use indexes for conditions like:
> +      WHERE field IN (const1, .. constN)
> +  */
> +  if (prefer_predicant_type_handler &&
> +      args->arguments()[value_index]->const_item() &&
> +      !args->arguments()[value_index]->is_expensive() &&
> +      tmp.type_handler()->cmp_type() == DECIMAL_RESULT &&
> +      args->arguments()[m_predicant_index]->cmp_type() == INT_RESULT)
> +  {
> +    /*
> +      For now we catch only one special case:
> +      an INT predicant can be compared to a DECIMAL constant
> +      using Longlong_hybrid comparison, when the DECIMAL constant:
> +      a. has no significant fractional digits, and
> +      b. is within the signed longlong range
> +      For DECIMAL constants in the range LONGLONG_MAX..ULONGLONG_MAX
> +      we cannot call to_longlong_hybrid() safely, because DECIMAL type
> +      Items usually set their "unsigned_flag" to "false", so val_int()
> +      will truncate such constants to LONGLONG_MAX (instead of ULONGLONG_MAX).
> +      TODO: fix to_longlong_hybrid() for DECIMAL LONGLONG_MAX..ULONGLONG_MAX
> +      TODO: move this code to Type_handler (will be done by MDEV-18898)
> +      TODO: skip constants outside of LONGLONG_MIN..ULONGLONG_MAX, as
> +            such conditon can never be true, e.g.:
> +             WHERE int_expr IN (.. -9223372036854775809 ..)
> +             WHERE int_expr IN (.. 18446744073709551616 ..)
> +    */
> +    my_decimal buf, *dec= args->arguments()[value_index]->val_decimal(&buf);
> +    longlong res;
> +    if (dec && decimal_actual_fraction(dec) == 0 &&
> +        my_decimal2int(0, dec, false /*SIGNED*/, &res) == 0)
> +      tmp.set_handler(&type_handler_longlong);

why not to replace the item with Item_int or Item_cache_int here?
Then you'll be able to handle decimals up to ULONGLONG_MAX, that's your
first TODO.

For the last TODO, you can simply remove the always-false element from
the arguments[] array.

> +  }
>    m_comparators[m_comparator_count].m_handler= tmp.type_handler();
>    m_comparators[m_comparator_count].m_arg_index= value_index;
>    m_comparator_count++;
> diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
> index 06f15503258a..4e84c3008578 100644
> --- a/sql/item_cmpfunc.h
> +++ b/sql/item_cmpfunc.h
> @@ -1588,29 +1588,28 @@ class cmp_item_sort_string :public cmp_item_string
>  
>  class cmp_item_int : public cmp_item_scalar
>  {
> -  longlong value;
> +  Longlong_hybrid m_value;
>  public:
> -  cmp_item_int() {}                           /* Remove gcc warning */
> +  cmp_item_int(): m_value(0, false) {}
>    void store_value(Item *item)
>    {
> -    value= item->val_int();
> +    m_value= item->to_longlong_hybrid();

I wonder if to_longlong_hybrid() returns a Longlong_hybrid like that,
where the compiler will allocate it? Could you check it, please?

>      m_null_value= item->null_value;
>    }

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx