[Maria-developers] core review #1
Hi, Shubham!
Here's a first review.
Summary: looks good so far, main comments:
1. Coding style. Use the same coding style as everywhere else in the
file you're editing. It's the same in sql/ and in myisam/ directories.
But InnoDB uses different coding style.
2. Tests, tests, tests! Please start adding tests now, and commit them
into your branch. They should be somewhere under mysql-test/ directory.
The full documentation is here: https://mariadb.com/kb/en/mariadb/mysqltest/
But even without it you can create your tests by starting of from
existing test files.
More comments below:
> diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> index dfce503..31f282b 100644
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -3876,8 +3876,9 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO
> *create_info,
> column->length= MAX_LEN_GEOM_POINT_FIELD;
> if (!column->length)
> {
> - my_error(ER_BLOB_KEY_WITHOUT_LENGTH, MYF(0),
> column->field_name.str);
> - DBUG_RETURN(TRUE);
> + key_info->algorithm=HA_KEY_ALG_HASH;
There's more to it. The task title is, indeed, "unique index for
blobs", but the actual goal is to have unique indexes for anything
that is too long for normal btree indexes. That's what MI_UNIQUE does.
so, you should support unique indexes also for blobs where column->length
is specified, but is too large. Or unique indexes for a combination of
ten long varchar columns, for example.
So, you also need to patch the code where it issues ER_TOO_LONG_KEY.
Also we'd want to use MI_UNIQUE for keys where a user has
explicitly specified USING HASH, but let's look at it later
> + // my_error(ER_BLOB_KEY_WITHOUT_LENGTH, MYF(0),
> column->field_name.str);
> + // DBUG_RETURN(TRUE);
> }
> }
> #ifdef HAVE_SPATIAL
> diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc
> index 72bc4c0..9aba216 100644
> --- a/storage/myisam/ha_myisam.cc
> +++ b/storage/myisam/ha_myisam.cc
> @@ -216,44 +216,59 @@ static void mi_check_print_msg(HA_CHECK *param, const
> char* msg_type,
> 0 OK
> !0 error code
> */
> -
> int table2myisam(TABLE *table_arg, MI_KEYDEF **keydef_out,
> - MI_COLUMNDEF **recinfo_out, uint *records_out)
> + MI_COLUMNDEF **recinfo_out,MI_UNIQUEDEF **uniquedef_out
> ,uint *records_out)
> {
> - uint i, j, recpos, minpos, fieldpos, temp_length, length;
> + uint i, j, recpos, minpos, fieldpos, temp_length, length, k, l, m;
>enum ha_base_keytype type= HA_KEYTYPE_BINARY;
>uchar *record;
>KEY *pos;
>MI_KEYDEF *keydef;
> + MI_UNIQUEDEF *uniquedef;
>MI_COLUMNDEF *recinfo, *recinfo_pos;
>HA_KEYSEG *keyseg;
>TABLE_SHARE *share= table_arg->s;
>uint options= share->db_options_in_use;
>DBUG_ENTER("table2myisam");
> + pos= table_arg->key_info;
> + share->uniques=0;
> + for (i= 0; i < share->keys; i++,pos++)
> + {
> +if(pos->algorithm==HA_KEY_ALG_HASH)
> +{
> + share->uniques++ ;
> +}
> + }
>if (!(my_multi_malloc(MYF(MY_WME),
>recinfo_out, (share->fields * 2 + 2) * sizeof(MI_COLUMNDEF),
> - keydef_out, share->keys * sizeof(MI_KEYDEF),
> + keydef_out, (share->keys - share->uniques) * sizeof(MI_KEYDEF),
> + uniquedef_out, share->uniques * sizeof(MI_UNIQUEDEF),
>&keyseg,
>(share->key_parts + share->keys) * sizeof(HA_KEYSEG),
>NullS)))
> DBUG_RETURN(HA_ERR_OUT_OF_MEM); /* purecov: inspected */
>keydef= *keydef_out;
>recinfo= *recinfo_out;
> + uniquedef= *uniquedef_out;
> pos= table_arg->key_info;
> + k=0;
> + m=0;
better call your indexes 'k' and 'u', for keydefs and uniques.
>for (i= 0; i < share->keys; i++, pos++)
>{
> -keydef[i].flag= ((uint16) pos->flags & (HA_NOSAME | HA_FULLTEXT |
> HA_SPATIAL));
> -keydef[i].key_alg= pos->algorithm == HA_KEY_ALG_UNDEF ?
> +if(pos->algorithm!=HA_KEY_ALG_HASH)
> +{
> +keydef[k].flag= ((uint16) pos->flags & (HA_NOSAME | HA_FULLTEXT |
> HA_SPATIAL));
> +keydef[k].key_alg= pos->algorithm == HA_KEY_ALG_UNDEF ?
>(pos->flags & HA_SPATIAL ? HA_KEY_ALG_RTREE : HA_KEY_ALG_BTREE) :
>pos->algorithm;
> -keydef[i].block_length= pos->block_size;
> -keydef[i].seg= keyseg;
> -keydef[i].keysegs= pos->user_defined_key_parts;
> +keydef[k].block_length= pos->block_size;
> +keydef[k].seg= keyseg;
> +keydef[k].keysegs= pos->user_defined_key_parts;
> for (j= 0; j < pos->user_defined_key_parts; j++)
> {
>Field *field= pos->key_part[j].field;
>type= field->key_type();
> - keydef[i].seg[j].flag= pos->key_part[j].key_part_flag;
> + keydef[k].seg[j].flag= pos->key_part[j].key_part_flag;
>
>if (options & HA_OPTION_PACK_KEYS ||
>(pos->flags & (HA_PACK_KEY | HA_BINARY_PACK_KEY |
> @@ -266,52 +281,104 @@ int table2myisam(TABLE *table_arg, M
[Maria-developers] Why Max Length of virtual column string is so small
Hello Everyone, Actually I was thinking why there is so small limit on virtual column string.Does it have any design implication or some other reason Regards sachin ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] core review #1
Hi Shubham,
On Sun, Jun 5, 2016 at 9:25 AM, Sergei Golubchik wrote:
> Hi, Shubham!
>
> Here's a first review.
>
> Summary: looks good so far, main comments:
>
> 1. Coding style. Use the same coding style as everywhere else in the
>file you're editing. It's the same in sql/ and in myisam/ directories.
>But InnoDB uses different coding style.
>
You can refer to the link below for coding guidelines.
https://dev.mysql.com/doc/internals/en/general-development-guidelines.html
Also, if you use vim, there are some good vim suggestions to fix
indentations.
Best,
Nirbhay
>
> 2. Tests, tests, tests! Please start adding tests now, and commit them
>into your branch. They should be somewhere under mysql-test/ directory.
>The full documentation is here:
> https://mariadb.com/kb/en/mariadb/mysqltest/
>But even without it you can create your tests by starting of from
>existing test files.
>
> More comments below:
>
> > diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> > index dfce503..31f282b 100644
> > --- a/sql/sql_table.cc
> > +++ b/sql/sql_table.cc
> > @@ -3876,8 +3876,9 @@ mysql_prepare_create_table(THD *thd,
> HA_CREATE_INFO *create_info,
> > column->length= MAX_LEN_GEOM_POINT_FIELD;
> > if (!column->length)
> > {
> > - my_error(ER_BLOB_KEY_WITHOUT_LENGTH, MYF(0),
> column->field_name.str);
> > - DBUG_RETURN(TRUE);
> > + key_info->algorithm=HA_KEY_ALG_HASH;
>
> There's more to it. The task title is, indeed, "unique index for
> blobs", but the actual goal is to have unique indexes for anything
> that is too long for normal btree indexes. That's what MI_UNIQUE does.
>
> so, you should support unique indexes also for blobs where column->length
> is specified, but is too large. Or unique indexes for a combination of
> ten long varchar columns, for example.
>
> So, you also need to patch the code where it issues ER_TOO_LONG_KEY.
>
> Also we'd want to use MI_UNIQUE for keys where a user has
> explicitly specified USING HASH, but let's look at it later
>
> > + // my_error(ER_BLOB_KEY_WITHOUT_LENGTH, MYF(0),
> column->field_name.str);
> > + // DBUG_RETURN(TRUE);
> > }
> > }
> > #ifdef HAVE_SPATIAL
> > diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc
> > index 72bc4c0..9aba216 100644
> > --- a/storage/myisam/ha_myisam.cc
> > +++ b/storage/myisam/ha_myisam.cc
> > @@ -216,44 +216,59 @@ static void mi_check_print_msg(HA_CHECK *param,
> const char* msg_type,
> > 0 OK
> > !0 error code
> > */
> > -
> > int table2myisam(TABLE *table_arg, MI_KEYDEF **keydef_out,
> > - MI_COLUMNDEF **recinfo_out, uint *records_out)
> > + MI_COLUMNDEF **recinfo_out,MI_UNIQUEDEF
> **uniquedef_out ,uint *records_out)
> > {
> > - uint i, j, recpos, minpos, fieldpos, temp_length, length;
> > + uint i, j, recpos, minpos, fieldpos, temp_length, length, k, l, m;
> >enum ha_base_keytype type= HA_KEYTYPE_BINARY;
> >uchar *record;
> >KEY *pos;
> >MI_KEYDEF *keydef;
> > + MI_UNIQUEDEF *uniquedef;
> >MI_COLUMNDEF *recinfo, *recinfo_pos;
> >HA_KEYSEG *keyseg;
> >TABLE_SHARE *share= table_arg->s;
> >uint options= share->db_options_in_use;
> >DBUG_ENTER("table2myisam");
> > + pos= table_arg->key_info;
> > + share->uniques=0;
> > + for (i= 0; i < share->keys; i++,pos++)
> > + {
> > +if(pos->algorithm==HA_KEY_ALG_HASH)
> > +{
> > + share->uniques++ ;
> > +}
> > + }
> >if (!(my_multi_malloc(MYF(MY_WME),
> >recinfo_out, (share->fields * 2 + 2) * sizeof(MI_COLUMNDEF),
> > - keydef_out, share->keys * sizeof(MI_KEYDEF),
> > + keydef_out, (share->keys - share->uniques) *
> sizeof(MI_KEYDEF),
> > + uniquedef_out, share->uniques * sizeof(MI_UNIQUEDEF),
> >&keyseg,
> >(share->key_parts + share->keys) * sizeof(HA_KEYSEG),
> >NullS)))
> > DBUG_RETURN(HA_ERR_OUT_OF_MEM); /* purecov: inspected */
> >keydef= *keydef_out;
> >recinfo= *recinfo_out;
> > + uniquedef= *uniquedef_out;
> > pos= table_arg->key_info;
> > + k=0;
> > + m=0;
>
> better call your indexes 'k' and 'u', for keydefs and uniques.
>
> >for (i= 0; i < share->keys; i++, pos++)
> >{
> > -keydef[i].flag= ((uint16) pos->flags & (HA_NOSAME | HA_FULLTEXT |
> HA_SPATIAL));
> > -keydef[i].key_alg= pos->algorithm == HA_KEY_ALG_UNDEF ?
> > +if(pos->algorithm!=HA_KEY_ALG_HASH)
> > +{
> > +keydef[k].flag= ((uint16) pos->flags & (HA_NOSAME | HA_FULLTEXT |
> HA_SPATIAL));
> > +keydef[k].key_alg= pos->algorithm == HA_KEY_ALG_UNDEF ?
> >(pos->flags & HA_SPATIAL ? HA_KEY_ALG_RTREE : HA_KEY_ALG_BTREE) :
> >pos->algorithm;
> > -keydef[i].block_length= pos->block_size;
> > -keydef[i].seg= keyseg;
> > -keydef[i].keysegs= pos->user_defined_key_parts;
> > +keydef[k].block_length= pos->block_size;
> > +keydef[k].
Re: [Maria-developers] InnoDB blob for primary key
Hello Sir Weekly Report for second week of gsoc 1. Implemented unique in the case of null 2. Implemented unique(A,b,c) Currently working on tests , field hiding Regards sachin On Tue, May 31, 2016 at 10:49 PM, Sergei Golubchik wrote: > Hi, Sachin! > > On May 31, Sachin Setia wrote: > > Hello Sir > > Weekly Report For Gsoc > > 1. Implemented hash function in parser for one column > > 2. Implemenetd hash function for unlimited columns in parser > > 3. Implemented unique checking in case of hash collusion(it retrieves the > > row and compare data) but this only woks for unique blob column > > > > Currently Working on > > unique checking in case of unique(a,b,c) > > Thanks. This is ok. > > Next time, please, send it to maria-developers list, not to me only :) > > Do you have questions? Any code you want me to look at? > > Regards, > Sergei > Chief Architect MariaDB > and secur...@mariadb.org > ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
