← Back to team overview

maria-developers team mailing list archive

Re: bbe056ac3fa: support NULL fields in key

 

On Wed, 11 Mar 2020 at 04:51, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Nikita!
>
> On Mar 10, Nikita Malyavin wrote:
> > revision-id: bbe056ac3fa (mariadb-10.5.0-273-gbbe056ac3fa)
> > parent(s): 7a5d3316805
> > author: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> > committer: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> > timestamp: 2020-03-11 00:46:24 +1000
> > message:
> >
> > support NULL fields in key
> >
> > ---
> >  mysql-test/suite/period/r/overlaps.result | 18 ++++++++++++++++--
> >  mysql-test/suite/period/t/overlaps.test   | 16 +++++++++++++++-
> >  sql/handler.cc                            | 18 ++++++++++++++----
> >  sql/sql_table.cc                          | 29
> +++++++----------------------
> >  4 files changed, 52 insertions(+), 29 deletions(-)
> >
> > diff --git a/mysql-test/suite/period/r/overlaps.result
> b/mysql-test/suite/period/r/overlaps.result
> > index cf980afd7f0..e52b21496b5 100644
> > --- a/mysql-test/suite/period/r/overlaps.result
> > +++ b/mysql-test/suite/period/r/overlaps.result
> > @@ -104,16 +104,30 @@ show create table t;
> >  Table        Create Table
> >  t    CREATE TABLE `t` (
> >    `id` int(11) NOT NULL,
> > -  `u` int(11) NOT NULL,
> > +  `u` int(11) DEFAULT NULL,
> >    `s` date NOT NULL,
> >    `e` date NOT NULL,
> >    PERIOD FOR `p` (`s`, `e`),
> >    PRIMARY KEY (`id`,`p` WITHOUT OVERLAPS),
> >    UNIQUE KEY `u` (`u`,`p` WITHOUT OVERLAPS)
> >  ) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1
> > +insert into t values (2, NULL, '2003-03-01', '2003-05-01');
> > +insert into t values (2, NULL, '2003-03-01', '2003-05-01');
> > +ERROR 23000: Duplicate entry '2-2003-05-01-2003-03-01' for key 'PRIMARY'
>
> This is wrong. Should be no duplicate key error above.
>
> No, 'tis correct: the table has two unique keys, both WITHOUT OVERLAPS, on
the same period. So it is a duplicate by a first (PRIMARY) key.

> > +insert into t values (3, NULL, '2003-03-01', '2003-05-01');
> >  insert into t values (1, 1, '2003-03-01', '2003-05-01');
> >  insert into t values (1, 2, '2003-05-01', '2003-07-01');
> > -insert into t values (2, 1, '2003-05-01', '2003-07-01');
> > +insert into t values (4, NULL, '2003-03-01', '2003-05-01');
> > +create sequence seq start=5;
> > +update t set id= nextval(seq), u= nextval(seq), s='2003-05-01',
> e='2003-07-01'
> > +         where u is NULL;
> > +select * from t;
> > +id   u       s       e
> > +1    1       2003-03-01      2003-05-01
> > +1    2       2003-05-01      2003-07-01
> > +5    6       2003-05-01      2003-07-01
> > +7    8       2003-05-01      2003-07-01
> > +9    10      2003-05-01      2003-07-01
> >  create or replace table t(id int, s date, e date,
> >  period for p(s,e));
> >  insert into t values (1, '2003-01-01', '2003-03-01'),
> > diff --git a/sql/handler.cc b/sql/handler.cc
> > index 917386f4392..16f57533f27 100644
> > --- a/sql/handler.cc
> > +++ b/sql/handler.cc
> > @@ -7015,7 +7017,8 @@ int handler::ha_check_overlaps(const uchar
> *old_data, const uchar* new_data)
> >        bool key_used= false;
> >        for (uint k= 0; k < key_parts && !key_used; k++)
> >          key_used= bitmap_is_set(table->write_set,
> > -                                key_info.key_part[k].fieldnr - 1);
> > +                                key_info.key_part[k].fieldnr - 1)
> > +                  &&
> !key_info.key_part[k].field->is_null_in_record(new_data);
>
> Why is that?
>
> >        if (!key_used)
> >          continue;
> >      }
> > @@ -7064,8 +7067,15 @@ int handler::ha_check_overlaps(const uchar
> *old_data, const uchar* new_data)
> >          error= handler->ha_index_next(record_buffer);
> >      }
> >
> > -    if (!error && table->check_period_overlaps(key_info, key_info,
> > -                                               new_data, record_buffer)
> == 0)
> > +    bool null_in_key= false;
> > +    for (uint k= 0; k < key_parts && !null_in_key; k++)
> > +    {
> > +      null_in_key=
> key_info.key_part[k].field->is_null_in_record(record_buffer);
> > +    }
> > +
> > +    if (!null_in_key && !error
> > +        && table->check_period_overlaps(key_info, key_info,
> > +                                        new_data, record_buffer) == 0)
>
> That's strange. You compare keys in key_period_compare_bases(), why do
> you do NULL values here?
>
> A record with NULL in key can be returned by ha_index_read_idx_map, or by
ha_index_next. It means bases are not equal. I didn't include the code in
table->check_period_overlaps, since we actually are not ever interested in
comparing key bases with NULLs inside. We only care iff key has NULL

> >        error= HA_ERR_FOUND_DUPP_KEY;
> >
> >      if (error == HA_ERR_KEY_NOT_FOUND || error == HA_ERR_END_OF_FILE)
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx
>


-- 
Yours truly,
Nikita Malyavin

References