On Wed, 11 Mar 2020 at 04:51, Sergei Golubchik <serg@mariadb.org> 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@gmail.com>
> committer: Nikita Malyavin <nikitamalyavin@gmail.com>
> 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@mariadb.org


--
Yours truly,
Nikita Malyavin