- developers - lists.mariadb.org

Re: [Maria-developers] Wording suggestion?
by Sergei Golubchik 30 Jan '18

30 Jan '18

Hi, Russell! On Jan 30, Russell J.T. Dyer wrote: > How about concurrent_assign ? I think "concurrent" already has a clear meaning, at least in MariaDB - operations are done "concurrently" if they're executed truly in parallel, usually in different threads. For example, both MyISAM and InnoDB allow concurrent SELECTs, but only InnoDB allows concurrent UPDATEs. I considered simultaneous_assign, as a slightly shorter alternative to simultaneous_assignment. But it looked somewhat unnatural to me. Still, this is the case when I happily yield to the opinion of a documentation writer :) If you think that simultaneous_assign looks just fine, I'll use it. > > It's not only for update. There is a patch to fix SELECT ... INTO, > > and I suppose we'll have "simultaneous assignment" behavior > > everywhere eventually. > > > > So, I'm leaning towards either simultaneous_assignment or > > simultaneous_set as mode names. The first is more exact, but it's > > rather long (sql_mode names range from 3 to 26 characters, and this > > one is 23). The second (with 16) fits right in the middle of the > > range, length-wise. But there's no SET keyword in SELECT ... INTO. > > On the other hand, there's no ASSIGNMENT keyword anywhere either :) > > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] Mdev-15085 Invisible Column Non-constant Default value results...
by Sergei Golubchik 30 Jan '18

30 Jan '18

Hi, Sachin! ok to push! On Jan 30, Sachin Setiya wrote: > commit a2e6d3a2fe6842bd3c35dffb1cd6f6d8a3117107 > Author: Sachin Setiya <sachin.setiya(a)mariadb.com> > Date: Tue Jan 30 14:04:05 2018 +0530 > > Mdev-15085 Invisible Column Non-constant Default value results... > > Problem:- If we create table field with dynamic default value then that > field always gets NULL value. > > Analyze:- This is because in fill_record we simple continue at Invisible > column because we though that share->default_values(default value is > always copied into table->record[0] before insert) will have a default > value for them(which is true for constant defaults , but not for dynamic > defaults). > > Solution:- We simple set all_fields_have_value to null , and this will > make call to update_default_fields (in the case of dynamic default), And > default expr will be evaluted and value will be set in field. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] Wording suggestion?
by Sergei Golubchik 30 Jan '18

30 Jan '18

Hi, Russell! On Jan 29, Russell J.T. Dyer wrote: > Is this mode related only to UPDATE and this feature? If so, maybe it > could be something like one of these: > > update_static > update_stasis > update_isolation > > These would focus on the fact that it's related just to UPDATE and that the > values held in a state temporarily, or in isolation of each other. Sorry, I should've said earlier. It's not only for update. There is a patch to fix SELECT ... INTO, and I suppose we'll have "simultaneous assignment" behavior everywhere eventually. So, I'm leaning towards either simultaneous_assignment or simultaneous_set as mode names. The first is more exact, but it's rather long (sql_mode names range from 3 to 26 characters, and this one is 23). The second (with 16) fits right in the middle of the range, length-wise. But there's no SET keyword in SELECT ... INTO. On the other hand, there's no ASSIGNMENT keyword anywhere either :) Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

[Maria-developers] Wording suggestion?
by Sergei Golubchik 30 Jan '18

30 Jan '18

Hi, Ian, Russell, As you know, UPDATE in MariaDB is executed left-to-right, say, in the case of UPDATE t1 SET a=b, b=a both columns will have the same value, old value of `b`. We're now introducing a new sql_mode to have standard compatible behavior, where all assignments are executed kind of "simultaneously" (and the statement above would swap the values). What could the name for this new sql_mode be? Regards, Sergei

2 2

Re: [Maria-developers] [Commits] Mdev-15085 Rebased to latest 10.3
by Sergei Golubchik 30 Jan '18

30 Jan '18

Hi, Sachin! On Jan 29, Sachin Setiya wrote: > commit 905806689979ae91c89115f18b107ebacbc489e2 > Author: Sachin Setiya <sachin.setiya(a)maridb.com> > Date: Fri Jan 26 22:58:34 2018 +0530 > > Mdev-15085 Invisible Column Non-constant Default value results... > > Problem:- If we create table field with dynamic default value then that > field always gets NULL value. > > Analyze:- This is because in fill_record we simple continue at Invisible > column because we though that share->default_values(default value is > always copied into table->record[0] before insert) will have a default > value for them(which is true for constant defaults , but not for dynamic > defaults). > > Solution:- In case of invisible field we will assign default value to 'value' > variable, but we do this only for columns which have non dynamic default value. > We do not have to worry for Auto_increment field , storage engine will take > of it. > > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 45efa63..17ff7eb 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -8382,7 +8382,12 @@ fill_record(THD *thd, TABLE *table, Field **ptr, > List<Item> &values, > > if (field->invisible) > { > - continue; > + if(field->flags & AUTO_INCREMENT_FLAG || > + !field->default_value) > + continue; > + else > + value= new (thd->mem_root) Item_default_value(thd, > + &thd->lex->select_lex.context); No-no, you cannot create items in fill_record(). It would create a new item for every inserted row. I think the only thing you need to do here (and it doesn't depend on AUTO_INCREMENT_FLAG or field->default_value), you need to set all_fields_have_values to false. Like if (field->invisible) { all_fields_have_values= false; continue; } Indeed, hidden columns did not get a value, so all_fields_have_values should be false. I'd expect the rest will work automatically :) > } > else > value=v++; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

Re: [Maria-developers] MDEV-14849 CREATE + ALTER with user-invisible columns produce ...
by Sergei Golubchik 29 Jan '18

29 Jan '18

Hi, Sachin! On Jan 29, Sachin Setiya wrote: > commit 5bd97f22094f3b6a658d503f98022ccd9076bb19 > Author: Sachin Setiya <sachin.setiya(a)maridb.com> > Date: Mon Jan 29 12:31:07 2018 +0530 > > MDEV-14849 CREATE + ALTER with user-invisible columns produce ... > > Problem:- > create or replace table t1 (pk int auto_increment primary key invisible, i int); > alter table t1 modify pk int invisible; > This last alter makes a invisible column which is not null and does not > have default value. > > Analysis:- > This is caused because our error check for NOT_NULL_FLAG and > NO_DEFAULT_VALUE_FLAG flag misses this sql_field , but this is not the fault > of error check :).Actually this field come via mysql_prepare_alter_table > and it does not have NO_DEFAULT_VALUE_FLAG flag turned on. (If it was create > table NO_DEFAULT_VALUE_FLAG would have turned on Column_definition::check) > and this would have generated error. > > Solution:- > I have moved the error check to kind last of mysql_prepare_create_table > because upto this point we have applied NO_DEFAULT_VALUE_FLAG to required > column. ok to push! Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] MDEV-14785 SYSTEM_INVISIBLE behaviour not consistent ---Initial Patch
by Sergei Golubchik 29 Jan '18

29 Jan '18

Hi, Sachin! On Jan 29, Sachin Setiya wrote: > commit 5e2f5aa78a5f53b000a2b9c6989c9ed7076e2527 > Author: Sachin Setiya <sachin.setiya(a)maridb.com> Note the typo in your email ^^^^^ > Date: Mon Jan 29 17:09:32 2018 +0530 > > MDEV-14785 SYSTEM_INVISIBLE behaviour not consistent > > TODO Commit message > > diff --git a/sql/item.h b/sql/item.h > index d178d9f..0072dfd 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -2907,11 +2907,17 @@ class Item_field :public Item_ident > bool check_vcol_func_processor(void *arg) > { > context= 0; > - if (field && (field->unireg_check == Field::NEXT_NUMBER)) > + if (field) > { > - // Auto increment fields are unsupported > - return mark_unsupported_function(field_name.str, arg, VCOL_FIELD_REF | VCOL_AUTO_INC); > + if (field->unireg_check == Field::NEXT_NUMBER) > + // Auto increment fields are unsupported > + return mark_unsupported_function(field_name.str, arg, > + VCOL_FIELD_REF | VCOL_AUTO_INC); > + if (field->invisible > INVISIBLE_USER) > + return mark_unsupported_function(field_name.str, arg, > + VCOL_FIELD_REF | VCOL_SYSTEM_INVISIBLE); Why would you do that, instead of hiding system invisible fields in CREATE TABLE? From the user point of view they aren't fields, they don't exist in the table, and the error should be "no such field", not "Function or expression 'on SYSTEM_INVISIBLE column' cannot be used" > } > + > return mark_unsupported_function(field_name.str, arg, VCOL_FIELD_REF); > } > bool set_fields_as_dependent_processor(void *arg) > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 17ff7eb..89ba2ed 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -8128,6 +8128,11 @@ fill_record(THD *thd, TABLE *table_arg, List<Item> &fields, List<Item> &values, > value=v++; > DBUG_ASSERT(value); > Field *rfield= field->field; > + if (rfield->invisible == INVISIBLE_SYSTEM) > + { > + my_error(ER_BAD_FIELD_ERROR, MYF(0), rfield->field_name.str, thd->where); The error is correct. But do you need to do it in fill_record()? I'd thought you can do it when the statement is prepared, once, not in fill_record() which is called for every row, many times per statement. > + goto err; > + } > TABLE* table= rfield->table; > if (table->next_number_field && > rfield->field_index == table->next_number_field->field_index) > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index d2fd2ce..e4971d7 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -16639,6 +16639,7 @@ Field *create_tmp_field_from_field(THD *thd, Field *org_field, > new_field->next_equal_field= NULL; > new_field->option_list= NULL; > new_field->option_struct= NULL; > + new_field->invisible= org_field->invisible; What is it for? It'd mean that temporary tables can have invisible fields. What bugs does this change fix? > } > return new_field; > } > diff --git a/sql/sql_update.cc b/sql/sql_update.cc > index 7b4938f..7bdedc7 100644 > --- a/sql/sql_update.cc > +++ b/sql/sql_update.cc > @@ -1497,6 +1497,9 @@ int mysql_multi_update_prepare(THD *thd) > TABLE_LIST *table_list= lex->query_tables; > TABLE_LIST *tl; > List<Item> *fields= &lex->select_lex.item_list; > + List_iterator<Item> it(*fields); > + Item *item; > + Item_field *field; > table_map tables_for_update; > bool update_view= 0; > /* > @@ -1564,7 +1567,21 @@ int mysql_multi_update_prepare(THD *thd) > { > DBUG_RETURN(TRUE); > } > - > + //Check if we got any INVISIBLE_SYSTEM field > + while ((item= it++)) > + { > + if (Item::FIELD_ITEM == item->type()) > + { > + field= (Item_field *)item; > + if (field->field->invisible == INVISIBLE_SYSTEM) > + { > + my_error(ER_BAD_FIELD_ERROR,MYF(0), > + field->field->field_name.str, > + field->field->table->s->table_name.str); > + DBUG_RETURN(TRUE); > + } > + } > + } Dunno. What if I do UPDATE ... SET normal_field = row_start you'll have an invisible field, but it's not updated, so should be ok, right? May be something like if (field->field->invisible == INVISIBLE_SYSTEM && bitmap_is_set(field->table->write_set, field->field_index)) { ... my_error } And why here? I'd think this belongs into check_fields() function. > thd->table_map_for_update= tables_for_update= get_table_map(fields); Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] MDEV-13790 UNHEX() of a somewhat complicated CONCAT() returns NULL
by Alexander Barkov 29 Jan '18

29 Jan '18

Hello Sergei, Please review a patch for MDEV-13790. It removes the attempt to search the best suitable buffer and just returns the result in "str" passed to val_str(). I'm quite sure this optimization gives nothing. In can be useful only under very rare conditions, but generates so many bugs. Moreover, in 10.3 (or 10.4) we'll work on reentrant items. So all Item_xxx::tmp_buffer will be gone anyway, together with this optimization. I think an optimization proposal. Why not to introduce a new method: class Item { public: .. virtual append_to_string(String *str); .. } So for example Field_varchar could write directly to "str" passed to Item_func_concat::val_str(). This will make sure that the first allocation is done on the target "str", and "str" will always be the best buffer instead of any Item_xxx::tmp_value. Thanks!

2 3

Re: [Maria-developers] 980a5c31880: MDEV-14572: Assertion `! is_set()' failed in Diagnostics_area::set_eof_status upon EXPLAIN UPDATE in PS
by Sergei Golubchik 28 Jan '18

28 Jan '18

Hi, Oleksandr! On Dec 22, Oleksandr Byelkin wrote: > revision-id: 980a5c318803d6aa7355e019869597976dcf19bf (mariadb-10.0.33-37-g980a5c31880) > parent(s): 042f763268c0f209e7c12e0a6a72bb5d204dfe29 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2017-12-22 14:00:10 +0100 > message: > > MDEV-14572: Assertion `! is_set()' failed in Diagnostics_area::set_eof_status upon EXPLAIN UPDATE in PS > > Restore EXPAIN flag in SELECT_LEX before execution multi-update by flag in LEX > (the same but in other way made before INSERT/DELETE/SELECT) Please explain what was the reason for the bug, why the status was set twice, and how your change fixes it. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] ee8755e3c51: MDEV-15012: ASAN: numerous test failures in PS
by Sergei Golubchik 26 Jan '18

26 Jan '18

Hi, Oleksandr! Nice one! Ok to push! On Jan 24, Oleksandr Byelkin wrote: > revision-id: ee8755e3c51a1da8fcf108ad0257a7e62fc94347 (mariadb-5.5.59-32-gee8755e3c51) > parent(s): 76577e1e2602f3c30859a176808c433a263e1b0a > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2018-01-24 14:42:52 +0100 > message: > > MDEV-15012: ASAN: numerous test failures in PS > > First roll back changes, then free Items which can lead to memory freeing. > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0