← Back to team overview

maria-developers team mailing list archive

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

 

Hi, Sergey!

All fixed, thanks!

On Dec 07, Sergey Petrunia wrote:
> 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.
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx


References