maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hello Sergei,

Thanks, please find an updated patch version in:

https://github.com/MariaDB/server/commits/bb-10.7-bar-uuid

See comments inline:

On 10/21/21 7:47 PM, Sergei Golubchik wrote:
> > > > diff --git a/plugin/type_uuid/mysql-test/type_uuid/type_uuid_memory.result b/plugin/type_uuid/mysql-test/type_uuid/type_uuid_memory.result
> > > > index 42bb74d4b01..eb7a51895b9 100644
> > > > --- a/plugin/type_uuid/mysql-test/type_uuid/type_uuid_memory.result
> > > > +++ b/plugin/type_uuid/mysql-test/type_uuid/type_uuid_memory.result
> > > > @@ -25,7 +25,7 @@ a
> > > >    00000000-0000-0000-0000-0000000000ff
> > > >    EXPLAIN SELECT * FROM t1 WHERE a='00000000-0000-0000-0000-0000000000ff';
> > > >    id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> > > > -1	SIMPLE	t1	ref	a	a	17	const	2	Using where
> > > > +1	SIMPLE	t1	ref	a	a	17	const	4	Using where
> > >
> > > why?
> >
> > Hash collision now happens differently, this affects the optimizer
> > estimation for certain values.
> ...
> > So it did not change to worse. Just a coincidence.
>
> I hope so, although in all changed test results it did change to worse.
> Not always doubled, but always increased.

I have added more tests into type_uuid_engines.inc,
flipping all bytes (not just the last one), and
with more total number of records.

On this data set EXPLAIN stably returns "2 rows"
in the test type_uuid_memory.


> > > > diff --git a/sql/sql_type_fixedbin.h b/sql/sql_type_fixedbin.h
> > > > index c6e3d20bcfa..5141cb9fad4 100644
> > > > --- a/sql/sql_type_fixedbin.h
> > > > +++ b/sql/sql_type_fixedbin.h
> > > > +  static ulong KEY_pack_flags(uint column_nr)
> > > > +  {
> > > > +    /*
> > > > +      Return zero by default. A particular data type can override
> > > > +      this method return some flags, e.g. HA_PACK_KEY to enable
> > > > +      key prefix compression.
> > >
> > > what if you change it later, will it make old tables corrupted?
> >
> > I did not check.
> > This method is used only in mysql_prepare_create_table().
> > Then flags are stored inside FRM. In theory the change should not
> > make old tables corrupted.
>
> I think this too. Just thought it might be good to verify it. E.g.,
> create a table, flip the flag, recompile and see if it opens.

I've added tests into type_uuid_myisam.test,
examining INDEX_LENGTH/DATA_LENGTH ratio for three tables:

- Made by CREATE TABLE (i.e. current, with HA_PACK_KEY enabled)
- Restored from std_data/t1packkey.frm   (with HA_PACK_KEY enabled)
- Restored from std_data/t1nopackkey.frm (without HA_PACK_KEY)

It opens t1nopackkey.frm without problems,
and detects that compression is really disabled.


> > Your question made me also think what should be the default.
> > Probably we should do it the other way around:
> > - enable compression by default
> > - disable compression in INET6
> > What do you think?
>
> Enable if NATIVE_LEN > 6? or >8 ?

I guess the minimum length to enable compression
is for the engine to decide.

Note,

Type_handler_string
Type_handler_varchar
Type_handler_blob_common

return HA_PACK_KEY unconditionally.

So if we want to enable compression by default,
FixedBinTypeStorage::KEY_pack_flags() should possibly
return HA_PACK_KEY unconditionally as well.


On the other hand, it's not necessarily to decide now.
We can collect more data types and decide later.


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