← Back to team overview

maria-developers team mailing list archive

Review for: MDEV-26938 Support descending indexes internally in InnoDB (server part)

 

Hi Serg,

There is some non-trivial input too, after all. Please find below.

> commit da2903f03de039693354176fda836e6642d6d0f0
> Author: Sergei Golubchik <serg@xxxxxxxxxxx>
> Date:   Wed Nov 24 16:50:21 2021 +0100
> 
>     MDEV-26938 Support descending indexes internally in InnoDB (server part)
>     
>     * preserve DESC index property in the parser
>     * store it in the frm (only for HA_KEY_ALG_BTREE)
>     * read it from the frm
>     * show it in SHOW CREATE
>     * skip DESC indexes in opt_range.cc and opt_sum.cc
>     * ORDER BY test
> 

== Trivial input ==

> diff --git a/sql/unireg.cc b/sql/unireg.cc
> index 2726b9a68c2..d6449eeca4c 100644
> --- a/sql/unireg.cc
> +++ b/sql/unireg.cc
> @@ -685,6 +685,14 @@ static uint pack_keys(uchar *keybuff, uint key_count, KEY *keyinfo,
>      DBUG_PRINT("loop", ("flags: %lu  key_parts: %d  key_part: %p",
>                          key->flags, key->user_defined_key_parts,
>                          key->key_part));
> +    /* For SPATIAL, FULLTEXT and HASH indexes (anything other than B-tree),
> +       ignore the ASC/DESC attribute of columns. */
> +    const uchar ha_reverse_sort= key->algorithm == HA_KEY_ALG_BTREE
> +      ? HA_REVERSE_SORT :
> +      (key->algorithm == HA_KEY_ALG_UNDEF &&
> +       !(key->flags & (HA_FULLTEXT | HA_SPATIAL)))
> +      ? HA_REVERSE_SORT : 0;

The above is hard to read for me. I think if-statement form would be more
readable:

 const uchar ha_reverse_sort= 0;
 if (key->algorithm == HA_KEY_ALG_BTREE || 
     (key->algorithm == HA_KEY_ALG_UNDEF && 
      !(key->flags & (HA_FULLTEXT | HA_SPATIAL))
   ha_reverse_sort= HA_REVERSE_SORT;


== Issue #1: ordered index scan is not handled in ha_partition ==

Testcase:

create table t10 (a int, b int, key(a desc)) partition by hash(a) partitions 4;
insert into t10 select seq, seq from seq_0_to_999;

MariaDB [test]> select * from t10 order by a limit 3;
+------+------+
| a    | b    |
+------+------+
|    3 |    3 |
|    7 |    7 |
|   11 |   11 |
+------+------+
3 rows in set (0.002 sec)

Correct output:

MariaDB [test]> select * from t10 use index() order by a limit 3;
+------+------+
| a    | b    |
+------+------+
|    0 |    0 |
|    1 |    1 |
|    2 |    2 |
+------+------+
3 rows in set (0.011 sec)

== Issue #2: extra warning ==

MariaDB [test]> create table t1 (a int, b int, key(a), key(a));
Query OK, 0 rows affected, 1 warning (0.016 sec)

MariaDB [test]> show warnings;
+-------+------+--------------------------------------------------------------------------------------+
| Level | Code | Message                                                                              |
+-------+------+--------------------------------------------------------------------------------------+
| Note  | 1831 | Duplicate index `a_2`. This is deprecated and will be disallowed in a future release |
+-------+------+--------------------------------------------------------------------------------------+                               
1 row in set (0.000 sec)                                                                                                              
 
This is ok.


MariaDB [test]> create table t2 (a int, b int, key(a), key(a desc));                                                                 
Query OK, 0 rows affected, 1 warning (0.004 sec)                                                                                      
                                                                                                                                      
MariaDB [test]> show warnings;
+-------+------+--------------------------------------------------------------------------------------+                               
| Level | Code | Message                                                                              |                               
+-------+------+--------------------------------------------------------------------------------------+                               
| Note  | 1831 | Duplicate index `a_2`. This is deprecated and will be disallowed in a future release |                               
+-------+------+--------------------------------------------------------------------------------------+                               
1 row in set (0.000 sec)                                                                                                              
 
This is probably not? MySQL doesn't produce this warning in this case.

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




Follow ups