- developers - lists.mariadb.org

[Maria-developers] MariaDB 5.5.28a release prep
by Sergei Golubchik 28 Nov '12

28 Nov '12

Preliminary release notes and changelog pages have been created for 5.5.28a https://kb.askmonty.org/en/mariadb-5528a-release-notes/ https://kb.askmonty.org/en/mariadb-5528a-changelog/ Please let me know if there is anything in particular which should be mentioned in the release notes. Thanks, Sergei _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers(a)lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp

1 0

[Maria-developers] Review of mdev-452
by Michael Widenius 27 Nov '12

27 Nov '12

Hi! Here is the review of https://mariadb.atlassian.net/browse/MDEV-452 (Add full support for auto-initialized/updated timestamp and datetime) === modified file 'mysql-test/r/log_tables.result' --- mysql-test/r/log_tables.result 2011-10-19 19:45:18 +0000 +++ mysql-test/r/log_tables.result 2012-10-17 12:43:56 +0000 @@ -53,7 +53,7 @@ ERROR HY000: You can't use locks with lo show create table mysql.general_log; Table Create Table general_log CREATE TABLE `general_log` ( - `event_time` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + `event_time` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), Why does all log tables now have CURRENT_TIMESTAMP(6) instead of CURRENT_TIMESTAMP ? This would cause a problem for someone that updates to MySQL 10.0 and then want to go back... Many of the other result file changes comes from the same change for general_log and slow_log. Is this also the case for MySQL 5.6 or only for MariaDB? Answer from Timour on IRC: Timour> This comes from MySQL 5.6 and we need to stay compatbile. === modified file 'sql/event_db_repository.cc' --- sql/event_db_repository.cc 2012-09-04 16:26:30 +0000 +++ sql/event_db_repository.cc 2012-10-17 12:43:56 +0000 @@ -829,9 +829,6 @@ Event_db_repository::update_event(THD *t (int) table->field[ET_FIELD_ON_COMPLETION]->val_int())) goto end; - /* Don't update create on row update. */ - table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET; - Why don't we anymore need to tell the rest of the code that the automatic fields shouldn't be updated from this function? Timour> As I remember, the main change compared to the old design is Timour> that we no longer update timestamps inside the handler Timour> So the place where timestamp updates are triggered is such Timour> that such updates will no longer be triggered in these cases. Yes, mysql_event_fill_row() doesn't call fill_record_n_invoke_before_triggers(). It instead has own code that probably not calling setting of any default fields. This does however mean that one can never get automatic timestamps with events. === modified file 'sql/field.cc' --- sql/field.cc 2012-09-08 22:22:06 +0000 +++ sql/field.cc 2012-10-17 12:43:56 +0000 @@ -4419,10 +4419,12 @@ Field_timestamp::Field_timestamp(uchar * { /* For 4.0 MYD and 4.0 InnoDB compatibility */ flags|= UNSIGNED_FLAG | BINARY_FLAG; - if (unireg_check != NONE && !share->timestamp_field) + if (unireg_check != NONE) { - /* This timestamp has auto-update */ - share->timestamp_field= this; + /* + This TIMESTAMP column is hereby quietly assumed to have an insert or + update default function. + */ We need to here add a comment what the TIMESTAMP_FLAG now means, as it has lost it's orginal meaning. Add to the above comment: /* We mark the flag with TIMESTAMP_FLAG to indicate to the client that this field will be automaticly updated on insert */ flags|= TIMESTAMP_FLAG; if (unireg_check != TIMESTAMP_DN_FIELD) flags|= ON_UPDATE_NOW_FLAG; Please also fix the function comment that refers to TABLE::timestamp_field <cut> +void Field_timestamp::set_explicit_default(Item *value) +{ + if (value && + ((value->type() == Item::DEFAULT_VALUE_ITEM && + !((Item_default_value*)value)->arg) || + (!maybe_null() && value->is_null()))) + return; + flags|= HAS_EXPLICIT_DEFAULT; +} The above function confused me greatly (mostly because of names) You should not have to check for value != 0. An item should never be 0. I noticed that some code called set_explicit_default() with NULL, but it would be better to for this add a general item function: field->set_explicit_default(NULL) -> field->mark_that_field_has_been_given_a_value() Where: Field::mark_that_field_has_been_given_a_value() { flags|= HAS_EXPLICIT_VALUE; } (Note name change for HAS_EXPLICIT_DEFAULT) This can be inline and we will win some function calls. The set_explicit_default() function also needs a comment as it's very confusing. To clarify the code, lets first add a function: /* Returns 1 if the default returns the default value for a column */ Item_default_value::default_column() { return arg != 0; } And then original function would look like: /** Mark columns that has been given a value with HAS_EXPLICIT_VALUE. For timestamp columns, the only case where a column is not marked with been given a value are: - It's explicitely assigned with DEFAULT - We assign NULL to a timestamp field that is defined as NOT NULL. This is how MySQL has worked since it's start. */ void Field_timestamp::set_explicit_default(Item *value) { if (((value->type() == Item::DEFAULT_VALUE_ITEM && !(Item_default_value*) value)->default_column()) || (!maybe_null() && value->is_null())) return; mark_that_field_has_been_given_a_value(); } <cut> + +int Field_datetime::set_time() +{ + THD *thd= current_thd; current_thd -> table->in_use; + MYSQL_TIME now_time; + thd->variables.time_zone->gmt_sec_to_TIME(&now_time, thd->query_start()); + now_time.second_part= thd->query_start_sec_part(); + set_notnull(); + store_TIME(&now_time); + thd->time_zone_used= 1; + return 0; +} @@ -8857,16 +8866,37 @@ bool Create_field::init(THD *thd, char * { uint sign_len, allowed_type_modifier= 0; ulong max_field_charlength= MAX_FIELD_CHARLENGTH; + const bool on_update_is_function= + (fld_on_update_value != NULL && + fld_on_update_value->type() == Item::FUNC_ITEM); DBUG_ENTER("Create_field::init()"); field= 0; field_name= fld_name; - def= fld_default_value; flags= fld_type_modifier; option_list= create_opt; - unireg_check= (fld_type_modifier & AUTO_INCREMENT_FLAG ? - Field::NEXT_NUMBER : Field::NONE); + + if (fld_default_value != NULL && fld_default_value->type() == Item::FUNC_ITEM) + { + /* There is a function default for insertions. */ + def= NULL; + unireg_check= on_update_is_function ? + Field::TIMESTAMP_DNUN_FIELD : // for insertions and for updates. + Field::TIMESTAMP_DN_FIELD; // only for insertions. Add braces around the on_update_is... to get better indentation. + } + else + { + /* No function default for insertions. Either NULL or a constant. */ + def= fld_default_value; + if (on_update_is_function) + unireg_check= Field::TIMESTAMP_UN_FIELD; // function default for updates + else + unireg_check= (fld_type_modifier & AUTO_INCREMENT_FLAG) != 0 ? + Field::NEXT_NUMBER : // Automatic increment. + Field::NONE; Add () around expression. + } + <cut> + + +/** + Mark the field as having an explicit default value. + + @param value if available, the value that the field is being set to + + @note + Fields that have an explicit default value should not be updated + automatically via the DEFAULT or ON UPDATE functions. The functions + that deal with data change functionality (INSERT/UPDATE/LOAD), + determine if there is an explicit value for each field before performing + the data change, and call this method to mark the field. + + If the 'value' parameter is NULL, then the field is marked unconditionally + as having an explicit value. If 'value' is not NULL, then it can be further + analyzed to check if it really should count as a value. +*/ + +void Field::set_explicit_default(Item *value) +{ /* Return if value is DEFAULT */ + if (value && value->type() == Item::DEFAULT_VALUE_ITEM && + !((Item_default_value*)value)->arg) + return; + flags|= HAS_EXPLICIT_DEFAULT; +} Remove checking if value is null. flags|= HAS_EXPLICIT_DEFAULT -> flags|= HAS_EXPLICIT_VALUE === modified file 'sql/field.h' @@ -1239,12 +1270,26 @@ class Field_timestamp :public Field_str virtual int set_time(); <cut> + virtual void set_explicit_default(Item *value); + virtual int evaluate_insert_default_function() + { + int res= 0; + if (has_insert_default_function()) + res= set_time(); + return res; + } + virtual int evaluate_update_default_function() + { + int res= 0; + if (has_update_default_function()) + res= set_time(); + return res; + } /* Get TIMESTAMP field value as seconds since begging of Unix Epoch */ virtual my_time_t get_timestamp(ulong *sec_part) const; virtual void store_TIME(my_time_t timestamp, ulong sec_part) @@ -1252,7 +1297,6 @@ class Field_timestamp :public Field_str int4store(ptr,timestamp); } bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate); - timestamp_auto_set_type get_auto_set_type() const; uchar *pack(uchar *to, const uchar *from, uint max_length __attribute__((unused))) { @@ -1503,6 +1547,28 @@ class Field_datetime :public Field_tempo void sql_type(String &str) const; bool zero_pack() const { return 1; } bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate); + virtual int set_time(); + virtual void set_default() + { + if (has_insert_default_function()) + set_time(); + else + Field::set_default(); + } + virtual int evaluate_insert_default_function() + { + int res= 0; + if (has_insert_default_function()) + res= set_time(); + return res; + } + virtual int evaluate_update_default_function() + { + int res= 0; + if (has_update_default_function()) + res= set_time(); + return res; + } It's a bit of a shame that we need to have the exact same functions twice? One way to solve it would be to add these functions to Field_str, as both Field_timestamp() and Field_datetime() depends on this class. === modified file 'sql/sql_base.cc' <cut> static bool -fill_record(THD * thd, List<Item> &fields, List<Item> &values, +fill_record(THD * thd, TABLE *table_arg, List<Item> &fields, List<Item> &values, bool ignore_errors) { Why this change (was not needed for this functionality). timour> I think I had a different implementation before, then I kept this timrou> change, because it looked simpler. bool -fill_record_n_invoke_before_triggers(THD *thd, List<Item> &fields, +fill_record_n_invoke_before_triggers(THD *thd, TABLE *table, List<Item> &fields, List<Item> &values, bool ignore_errors, - Table_triggers_list *triggers, enum trg_event_type event) { bool result; - result= (fill_record(thd, fields, values, ignore_errors) || + Table_triggers_list *triggers= table->triggers; + result= (fill_record(thd, table, fields, values, ignore_errors) || (triggers && triggers->process_triggers(thd, event, TRG_ACTION_BEFORE, TRUE))); /* @@ -8925,7 +8924,6 @@ fill_record_n_invoke_before_triggers(THD */ if (!result && triggers) { - TABLE *table= 0; List_iterator_fast<Item> f(fields); Item *fld; Item_field *item_field; @@ -8933,9 +8931,7 @@ fill_record_n_invoke_before_triggers(THD { fld= (Item_field*)f++; item_field= fld->filed_for_view_update(); - if (item_field && item_field->field && - (table= item_field->field->table) && - table->vfield) + if (item_field && item_field->field && table && table->vfield) result= update_virtual_fields(thd, table, TRUE); Add: DBUG_ASSERT(table == item_field->field->table) Note that if we can only have one table here, than we could also replace: if (!result && triggers) with: if (!result && triggers && table) As the loop will not do anything if table == 0. <cut> bool -fill_record_n_invoke_before_triggers(THD *thd, Field **ptr, +fill_record_n_invoke_before_triggers(THD *thd, TABLE *table, Field **ptr, List<Item> &values, bool ignore_errors, - Table_triggers_list *triggers, enum trg_event_type event) { bool result; - result= (fill_record(thd, ptr, values, ignore_errors, FALSE) || + Table_triggers_list *triggers= table->triggers; + result= (fill_record(thd, table, ptr, values, ignore_errors, FALSE) || (triggers && triggers->process_triggers(thd, event, TRG_ACTION_BEFORE, TRUE))); /* @@ -9071,7 +9064,6 @@ fill_record_n_invoke_before_triggers(THD */ if (!result && triggers && *ptr) { - TABLE *table= (*ptr)->table; Add DBUG_ASSERT(table == (*ptr)->table); if (table->vfield) result= update_virtual_fields(thd, table, TRUE); } <cut> === modified file 'sql/sql_insert.cc' @@ -1696,13 +1660,32 @@ int write_record(THD *thd, TABLE *table, restore_record(table,record[1]); DBUG_ASSERT(info->update_fields->elements == info->update_values->elements); - if (fill_record_n_invoke_before_triggers(thd, *info->update_fields, + if (fill_record_n_invoke_before_triggers(thd, table, *info->update_fields, *info->update_values, info->ignore, - table->triggers, TRG_EVENT_UPDATE)) goto before_trg_err; + bool different_records= (!records_are_comparable(table) || + compare_record(table)); + /* + Default fields must be updated before checking view updateability. + This branch of INSERT is executed only when a UNIQUE key was violated + with the ON DUPLICATE KEY UPDATE option. In this case the INSERT + operation is transformed to an UPDATE, and the default fields must + be updated as if this is an UPDATE. + */ + if (different_records && table->default_field) + { + bool res; + enum_sql_command cmd= thd->lex->sql_command; + thd->lex->sql_command= SQLCOM_UPDATE; + res= table->update_default_fields(); + thd->lex->sql_command= cmd; + if (res) + goto err; + } Another way to do the above nicer would be to give thd->lex->sql_command as an argument to update_default_fields(). Another option would be to have: update_defaults_fields(table) { return update_defaults_fields(table, table->in_use->lex->cmd); } And use the later in this case. However this is not that critical to fix... <cut> + Field **field,**org_field, *found_next_number_field, **dfield_ptr; You need to do dfield_ptr= 0 to avoid a compiler warning. @@ -2390,6 +2370,12 @@ TABLE *Delayed_insert::get_local_table(T bitmap= (uchar*) (field + share->fields + 1); copy->record[0]= (bitmap + share->column_bitmap_size*3); memcpy((char*) copy->record[0], (char*) table->record[0], share->reclength); + if (share->default_fields) + { + copy->default_field= (Field**) client_thd->alloc((share->default_fields+1)* + sizeof(Field**)); You should check for error condition here. + dfield_ptr= copy->default_field; + } /* Make a copy of all fields. The copied fields need to point into the copied record. This is done @@ -2407,18 +2393,19 @@ TABLE *Delayed_insert::get_local_table(T (*field)->move_field_offset(adjust_ptrs); // Point at copy->record[0] if (*org_field == found_next_number_field) (*field)->table->found_next_number_field= *field; + if (share->default_fields && + ((*org_field)->has_insert_default_function() || + (*org_field)->has_update_default_function())) + { + /* Put the newly copied field into the set of default fields. */ + *dfield_ptr= *field; + (*dfield_ptr)->unireg_check= (*org_field)->unireg_check; It's strange that new_field() resets unireg_check. Don't know why this is done and we should at some point fix that it's not reset and see what happens. + dfield_ptr++; + } } *field=0; <cut> @@ -850,7 +837,7 @@ read_fixed_length(THD *thd, COPY_INFO &i ER(ER_WARN_TOO_FEW_RECORDS), thd->warning_info->current_row_for_warning()); if (!field->maybe_null() && field->type() == FIELD_TYPE_TIMESTAMP) - ((Field_timestamp*) field)->set_time(); + field->set_time(); Shouldn't the above be done now by testing for if the field has an insert_default_function? @@ -994,10 +982,11 @@ read_sep_field(THD *thd, COPY_INFO &info DBUG_RETURN(1); } field->set_null(); + field->set_explicit_default(NULL); if (!field->maybe_null()) { if (field->type() == MYSQL_TYPE_TIMESTAMP) - ((Field_timestamp*) field)->set_time(); + field->set_time(); shouldn't we use an insert_default_function here too ? else if (field != table->next_number_field) field->set_warning(MYSQL_ERROR::WARN_LEVEL_WARN, ER_WARN_NULL_TO_NOTNULL, 1); @@ -1066,7 +1056,7 @@ read_sep_field(THD *thd, COPY_INFO &info DBUG_RETURN(1); } if (!field->maybe_null() && field->type() == FIELD_TYPE_TIMESTAMP) - ((Field_timestamp*) field)->set_time(); + field->set_time(); shouldn't we use an insert_default_function here too ? /* TODO: We probably should not throw warning for each field. But how about intention to always have the same number @@ -1206,7 +1196,7 @@ read_xml_field(THD *thd, COPY_INFO &info if (!field->maybe_null()) { if (field->type() == FIELD_TYPE_TIMESTAMP) - ((Field_timestamp *) field)->set_time(); + field->set_time(); shouldn't we use an insert_default_function here too ? === modified file 'sql/sql_parse.cc' --- sql/sql_parse.cc 2012-10-06 07:30:52 +0000 +++ sql/sql_parse.cc 2012-10-18 12:57:12 +0000 @@ -269,12 +269,14 @@ void init_update_queries(void) CF_CAN_GENERATE_ROW_EVENTS; sql_command_flags[SQLCOM_CREATE_INDEX]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_ALTER_TABLE]= CF_CHANGES_DATA | CF_WRITE_LOGS_COMMAND | - CF_AUTO_COMMIT_TRANS | CF_REPORT_PROGRESS ; + CF_AUTO_COMMIT_TRANS | CF_REPORT_PROGRESS | + CF_INSERTS_DATA; Should we really have CF_INSERTS_DATA here? We should not update any timestamps for ALTER TABLE? === modified file 'sql/table.cc' @@ -2472,7 +2471,7 @@ int open_table_from_share(THD *thd, TABL } /* - Process virtual columns, if any. + Process virtual and default columns, if any. */ if (!share->vfields) outparam->vfield= NULL; @@ -2484,10 +2483,26 @@ int open_table_from_share(THD *thd, TABL goto err; outparam->vfield= vfield_ptr; + } + + if (!share->default_fields) + outparam->default_field= NULL; You don't need to set default_fields to NULL as outparam is zeroed in beginning of the function. (Same is true for outparam->vfield) <cut> Regards, Monty

1 0

[Maria-developers] EXISTS-to-IN can work anywhere?
by Sergei Petrunia 26 Nov '12

26 Nov '12

Hello, Currently, EXISTS-to-IN rewrite only works on WHERE's top level, or under NOT, where the rewrite doesn't change semantics wrt NULL values. It seems, it is possible to make it work in other contexts, too. We need to convert it into a "NULL-ignoring IN", a special kind of IN predicate. For normal IN: NULL IN (select that returns something) --> NULL NULL IN (select that returns nothing) --> FALSE $value IN (select that doesn't return $value) --> FALSE $value IN (select that doesn't return $value but returns NULL) --> NULL null-ignoring-IN should return FALSE in all of the above cases. In fact, the optimizer is perfectly capable of doing this. What we need is just to disable special handling for NULLs. Let's discuss this tomorrow (Tuesday). BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] MariaDB 5.1.66 release prep
by Sergei Golubchik 26 Nov '12

26 Nov '12

Preliminary release notes and changelog pages have been created for 5.1.66, 5.2.13, 5.3.11. https://kb.askmonty.org/en/mariadb-5166-release-notes/ https://kb.askmonty.org/en/mariadb-5166-changelog/ https://kb.askmonty.org/en/mariadb-5213-release-notes/ https://kb.askmonty.org/en/mariadb-5213-changelog/ https://kb.askmonty.org/en/mariadb-5311-release-notes/ https://kb.askmonty.org/en/mariadb-5311-changelog/ Please let me know if there is anything in particular which should be mentioned in the release notes. Thanks, Sergei

1 0

[Maria-developers] EXISTS-to-IN is ok to push
by Sergei Petrunia 23 Nov '12

23 Nov '12

Hi Sanja, I've made some fixes to comments in lp:~maria-captains/maria/10.0-exists2in. With that, it is ok to push EXISTS-to-IN into 10.0. Sorry for the delay. BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] crash in TC_LOG_MMAP::log_one_transaction in maria-10.0.0
by Rich Prohaska 19 Nov '12

19 Nov '12

Hello, I am having problems with the shinny new mariadb-10.0.0. I have a simple test case that replaces a row in innodb and tokudb and then commits. Eventually, mysqld hits this assert: mysqld: /home/prohaska/maria10-build/mariadb-10.0.0/sql/log.cc:7547: int TC_LOG_MMAP::log_one_transaction(my_xid): Assertion `p->ptr < p->end' failed. The test works great when the binlog is enabled. The test crashes when the binlog is OFF. So, maybe the problem is isolated to the TC_LOG_MMAP logic. I noticed that the TC_LOG_MMAP::unlog function is quite different than the mariadb-5.5 version. What is the best way to get this fixed? Would it help if i made the test case available? Thanks Rich Prohaska Here is the stack trace: Program received signal SIGABRT, Aborted. 0x0000003006a32885 in raise () from /lib64/libc.so.6 (gdb) bt #0 0x0000003006a32885 in raise () from /lib64/libc.so.6 #1 0x0000003006a34065 in abort () from /lib64/libc.so.6 #2 0x0000003006a2b9fe in __assert_fail_base () from /lib64/libc.so.6 #3 0x0000003006a2bac0 in __assert_fail () from /lib64/libc.so.6 #4 0x00000000008862bf in TC_LOG_MMAP::log_one_transaction (this=0x1541f00 <tc_log_mmap>, xid=142018) at /home/prohaska/maria10-build/mariadb-10.0.0/sql/log.cc:7547 #5 0x0000000000885442 in TC_LOG_MMAP::log_and_order (this=0x1541f00 <tc_log_mmap>, thd=0x27f0000, xid=142018, all=true, need_prepare_ordered=false, need_commit_ordered=true) at /home/prohaska/maria10-\ build/mariadb-10.0.0/sql/log.cc:7180 #6 0x00000000007b819f in ha_commit_trans (thd=0x27f0000, all=true) at /home/prohaska/maria10-build/mariadb-10.0.0/sql/handler.cc:1329 #7 0x0000000000710307 in trans_commit (thd=0x27f0000) at /home/prohaska/maria10-build/mariadb-10.0.0/sql/transaction.cc:213 #8 0x000000000060a2df in mysql_execute_command (thd=0x27f0000) at /home/prohaska/maria10-build/mariadb-10.0.0/sql/sql_parse.cc:4084 #9 0x000000000060f1a7 in mysql_parse (thd=0x27f0000, rawbuf=0x7fff6c0050b8 "commit", length=6, parser_state=0x7fffe4112660) at /home/prohaska/maria10-build/mariadb-10.0.0/sql/sql_parse.cc:6056 #10 0x00000000006029a3 in dispatch_command (command=COM_QUERY, thd=0x27f0000, packet=0x27fa341 "commit", packet_length=6) at /home/prohaska/maria10-build/mariadb-10.0.0/sql/sql_parse.cc:1216 #11 0x0000000000601bdb in do_command (thd=0x27f0000) at /home/prohaska/maria10-build/mariadb-10.0.0/sql/sql_parse.cc:945 #12 0x00000000006ff6d1 in do_handle_one_connection (thd_arg=0x27f0000) at /home/prohaska/maria10-build/mariadb-10.0.0/sql/sql_connect.cc:1254 #13 0x00000000006ff179 in handle_one_connection (arg=0x27f0000) at /home/prohaska/maria10-build/mariadb-10.0.0/sql/sql_connect.cc:1168 #14 0x0000003006e077f1 in start_thread () from /lib64/libpthread.so.0 #15 0x0000003006ae592d in clone () from /lib64/libc.so.6

3 9

Re: [Maria-developers] Fwd: [Commits] Rev 3479: MDEV-3858 Change JOIN_TAB::records_read from ha_rows to double in file:///home/tsk/mprog/src/10.0-md3858/
by Sergei Petrunia 15 Nov '12

15 Nov '12

Hi Timour, Ok to push. On Thu, Nov 15, 2012 at 12:56:59PM +0200, Timour Katchaounov wrote: > Sergey, > > Could you please review this trivial task, shouldn't take more than 10 min. > > Timour > > ------------------------------------------------------------ > revno: 3479 > revision-id: timour(a)askmonty.org-20121115105450-zkiva73utg8pg49j > parent: wlad(a)montyprogram.com-20121111095852-1jsqvi3zcdml80lt > fixes bug: https://mariadb.atlassian.net/browse/MDEV-3858 > committer: timour(a)askmonty.org > branch nick: 10.0-md3858 > timestamp: Thu 2012-11-15 12:54:50 +0200 > message: > MDEV-3858 Change JOIN_TAB::records_read from ha_rows to double > > Currently JOIN_TAB::records_read is of type ha_rows. > This is an integer type, which prevents proper selectivity and rows estimates. > > > === modified file 'mysql-test/r/subselect_sj_mat.result' > --- a/mysql-test/r/subselect_sj_mat.result 2012-07-25 16:53:49 +0000 > +++ b/mysql-test/r/subselect_sj_mat.result 2012-11-15 10:54:50 +0000 > @@ -94,7 +94,7 @@ a1 a2 > explain extended > select * from t1i where a1 in (select b1 from t2i where b1 > '0'); > id select_type table type possible_keys key key_len ref rows filtered Extra > -1 PRIMARY t2i index it2i1,it2i3 it2i1 # NULL 5 40.00 Using where; Using index; LooseScan > +1 PRIMARY t2i index it2i1,it2i3 it2i1 # NULL 5 50.00 Using where; Using index; LooseScan > 1 PRIMARY t1i ref _it1_idx _it1_idx # _ref_ 1 100.00 > Warnings: > Note 1003 select `test`.`t1i`.`a1` AS `a1`,`test`.`t1i`.`a2` AS `a2` from `test`.`t1i` semi join (`test`.`t2i`) where ((`test`.`t1i`.`a1` = `test`.`t2i`.`b1`) and (`test`.`t2i`.`b1` > '0')) > @@ -117,7 +117,7 @@ a1 a2 > explain extended > select * from t1i where (a1, a2) in (select b1, b2 from t2i where b1 > '0'); > id select_type table type possible_keys key key_len ref rows filtered Extra > -1 PRIMARY t2i index it2i1,it2i2,it2i3 it2i3 # NULL 5 40.00 Using where; Using index; LooseScan > +1 PRIMARY t2i index it2i1,it2i2,it2i3 it2i3 # NULL 5 50.00 Using where; Using index; LooseScan > 1 PRIMARY t1i ref _it1_idx _it1_idx # _ref_ 1 100.00 > Warnings: > Note 1003 select `test`.`t1i`.`a1` AS `a1`,`test`.`t1i`.`a2` AS `a2` from `test`.`t1i` semi join (`test`.`t2i`) where ((`test`.`t1i`.`a2` = `test`.`t2i`.`b2`) and (`test`.`t1i`.`a1` = `test`.`t2i`.`b1`) and (`test`.`t2i`.`b1` > '0')) > @@ -319,7 +319,7 @@ where (a1, a2) in (select b1, b2 from t2 > (a1, a2) in (select c1, c2 from t3i > where (c1, c2) in (select b1, b2 from t2i where b2 > '0')); > id select_type table type possible_keys key key_len ref rows filtered Extra > -1 PRIMARY t2i index it2i1,it2i2,it2i3 # # # 5 40.00 # > +1 PRIMARY t2i index it2i1,it2i2,it2i3 # # # 5 50.00 # > 1 PRIMARY t1i ref it1i1,it1i2,it1i3 # # # 1 100.00 # > 1 PRIMARY t3i ref it3i1,it3i2,it3i3 # # # 1 100.00 # > 1 PRIMARY t2i ref it2i1,it2i2,it2i3 # # # 2 100.00 # > @@ -407,7 +407,7 @@ id select_type table type possible_keys > 2 MATERIALIZED t2 ALL NULL # # # 5 100.00 # > 4 MATERIALIZED t3 ALL NULL # # # 4 100.00 # > 3 MATERIALIZED t3 ALL NULL # # # 4 100.00 # > -7 UNION t2i index it2i1,it2i2,it2i3 # # # 5 40.00 # > +7 UNION t2i index it2i1,it2i2,it2i3 # # # 5 50.00 # > 7 UNION t1i ref it1i1,it1i2,it1i3 # # # 1 100.00 # > 7 UNION t3i ref it3i1,it3i2,it3i3 # # # 1 100.00 # > 7 UNION t2i ref it2i1,it2i2,it2i3 # # # 2 100.00 # > > === modified file 'sql/item_subselect.cc' > --- a/sql/item_subselect.cc 2012-11-03 11:28:51 +0000 > +++ b/sql/item_subselect.cc 2012-11-15 10:54:50 +0000 > @@ -4553,7 +4553,7 @@ double get_fanout_with_deps(JOIN *join, > !tab->emb_sj_nest && > tab->records_read != 0) > { > - fanout *= rows2double(tab->records_read); > + fanout *= tab->records_read; > } > } > return fanout; > > === modified file 'sql/sql_select.cc' > --- a/sql/sql_select.cc 2012-11-04 15:32:32 +0000 > +++ b/sql/sql_select.cc 2012-11-15 10:54:50 +0000 > @@ -6789,7 +6789,7 @@ void JOIN::get_prefix_cost_and_fanout(ui > > double JOIN::get_examined_rows() > { > - ha_rows examined_rows; > + double examined_rows; > double prev_fanout= 1; > JOIN_TAB *tab= first_breadth_first_tab(this, WALK_OPTIMIZATION_TABS); > JOIN_TAB *prev_tab= tab; > @@ -6799,7 +6799,7 @@ double JOIN::get_examined_rows() > while ((tab= next_breadth_first_tab(this, WALK_OPTIMIZATION_TABS, tab))) > { > prev_fanout *= prev_tab->records_read; > - examined_rows+= (ha_rows) (tab->get_examined_rows() * prev_fanout); > + examined_rows+= tab->get_examined_rows() * prev_fanout; > prev_tab= tab; > } > return examined_rows; > @@ -7704,7 +7704,8 @@ get_best_combination(JOIN *join) > sub-order > */ > SJ_MATERIALIZATION_INFO *sjm= cur_pos->table->emb_sj_nest->sj_mat_info; > - j->records= j->records_read= (ha_rows)(sjm->is_sj_scan? sjm->rows : 1); > + j->records_read= sjm->is_sj_scan? sjm->rows : 1; > + j->records= (ha_rows) j->records_read; > JOIN_TAB *jt; > JOIN_TAB_RANGE *jt_range; > if (!(jt= (JOIN_TAB*)join->thd->alloc(sizeof(JOIN_TAB)*sjm->tables)) || > @@ -7766,7 +7767,7 @@ get_best_combination(JOIN *join) > Save records_read in JOIN_TAB so that select_describe()/etc don't have > to access join->best_positions[]. > */ > - j->records_read= (ha_rows)join->best_positions[tablenr].records_read; > + j->records_read= join->best_positions[tablenr].records_read; > join->map2table[j->table->tablenr]= j; > > /* If we've reached the end of sjm nest, switch back to main sequence */ > @@ -10460,7 +10461,7 @@ double JOIN_TAB::scan_time() > > ha_rows JOIN_TAB::get_examined_rows() > { > - ha_rows examined_rows; > + double examined_rows; > > if (select && select->quick && use_quick != 2) > examined_rows= select->quick->records; > @@ -10490,7 +10491,7 @@ ha_rows JOIN_TAB::get_examined_rows() > } > } > else > - examined_rows= (ha_rows) records_read; > + examined_rows= records_read; > > return examined_rows; > } > @@ -21972,7 +21973,7 @@ int JOIN::print_explain(select_result_si > } > else > { > - ha_rows examined_rows= tab->get_examined_rows(); > + double examined_rows= tab->get_examined_rows(); > > item_list.push_back(new Item_int((longlong) (ulonglong) examined_rows, > MY_INT64_NUM_DECIMAL_DIGITS)); > @@ -21982,7 +21983,7 @@ int JOIN::print_explain(select_result_si > { > float f= 0.0; > if (examined_rows) > - f= (float) (100.0 * tab->records_read / examined_rows); > + f= (100.0 * (float)tab->records_read) / examined_rows; > set_if_smaller(f, 100.0); > item_list.push_back(new Item_float(f, 2)); > } > > === modified file 'sql/sql_select.h' > --- a/sql/sql_select.h 2012-11-04 15:32:32 +0000 > +++ b/sql/sql_select.h 2012-11-15 10:54:50 +0000 > @@ -280,8 +280,8 @@ typedef struct st_join_table { > */ > double read_time; > > - /* psergey-todo: make the below have type double, like POSITION::records_read? */ > - ha_rows records_read; > + /* Copy of POSITION::records_read, set by get_best_combination() */ > + double records_read; > > /* Startup cost for execution */ > double startup_cost; > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] The patch for the task mdev-539
by Sergei Golubchik 13 Nov '12

13 Nov '12

Hi, Igor! Sorry for a delay :( Please, find my review attached. On Sep 27, Igor Babaev wrote: > === modified file 'mysql-test/r/alter_table.result' > --- a/mysql-test/r/alter_table.result 2012-05-21 13:30:25 +0000 > +++ b/mysql-test/r/alter_table.result 2012-09-27 21:29:26 +0000 > @@ -1340,3 +1340,203 @@ > execute stmt1; > deallocate prepare stmt1; > drop table t2; > +# > +# mdev-539: fast build of unique/primary indexes for MyISAM > +# I'd suggest to have these test cases in a separate file, say, alter_table_539.test But it's just a siggestion, do as you prefer. > +DROP DATABASE IF EXISTS dbt3_s001; > +CREATE DATABASE dbt3_s001; > +use dbt3_s001; > +drop index `primary` on customer; > +show create table customer; > +Table Create Table > +customer CREATE TABLE `customer` ( > + `c_custkey` int(11) NOT NULL, > + `c_name` varchar(25) DEFAULT NULL, > + `c_address` varchar(40) DEFAULT NULL, > + `c_nationkey` int(11) DEFAULT NULL, > + `c_phone` char(15) DEFAULT NULL, > + `c_acctbal` double DEFAULT NULL, > + `c_mktsegment` char(10) DEFAULT NULL, > + `c_comment` varchar(117) DEFAULT NULL, > + KEY `i_c_nationkey` (`c_nationkey`) > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +alter table customer add primary key (c_custkey); > +show create table customer; > +Table Create Table > +customer CREATE TABLE `customer` ( > + `c_custkey` int(11) NOT NULL, > + `c_name` varchar(25) DEFAULT NULL, > + `c_address` varchar(40) DEFAULT NULL, > + `c_nationkey` int(11) DEFAULT NULL, > + `c_phone` char(15) DEFAULT NULL, > + `c_acctbal` double DEFAULT NULL, > + `c_mktsegment` char(10) DEFAULT NULL, > + `c_comment` varchar(117) DEFAULT NULL, > + PRIMARY KEY (`c_custkey`), > + KEY `i_c_nationkey` (`c_nationkey`) > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +drop index `primary` on customer; > +insert into customer values > +(3,'Customer#00000000303','MG9kdTD2WBHm',1,'11-719-748-3364',7498.12,'AUTOMOBILE','special packages wake. slyly reg'); > +alter table customer add primary key (c_custkey); please add a test case for alter ignore table > +ERROR 23000: Duplicate entry '3' for key 'PRIMARY' > +show create table customer; > +Table Create Table > +customer CREATE TABLE `customer` ( ... > === modified file 'sql/ha_partition.cc' > --- a/sql/ha_partition.cc 2012-08-31 21:54:54 +0000 > +++ b/sql/ha_partition.cc 2012-09-27 21:29:26 +0000 > @@ -6862,6 +6862,8 @@ > > flags_to_return= ht->alter_table_flags(flags); > flags_to_return|= m_file[0]->alter_table_flags(flags); > + flags_to_return&= > + ~(HA_FAST_ADD_INDEX | HA_FAST_ADD_UNIQUE_INDEX | HA_FAST_ADD_PK_INDEX); You don't need these flags - see below > > /* > If one partition fails we must be able to revert the change for the other, > > === modified file 'sql/sql_table.cc' > --- a/sql/sql_table.cc 2012-09-02 02:41:38 +0000 > +++ b/sql/sql_table.cc 2012-09-27 21:29:26 +0000 > @@ -6743,6 +6745,11 @@ > my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0)); > goto err_new_table_cleanup; > }); > + > + if (!ignore && > + test(alter_flags & (HA_FAST_ADD_UNIQUE_INDEX | HA_FAST_ADD_PK_INDEX))) > + new_table->file->extra(HA_EXTRA_CREATE_UNIQUE_INDEX_BY_SORT); What's the point? extra() functions were always *optional* in the storage engine API, any storage engine is allowed to ignore any HA_EXTRA_* requests, or all of them. It means, you don't need to introduce HA_FAST_whatever flags, and don't need to check them. Just write if (!ignore) new_table->file->extra(HA_EXTRA_CREATE_UNIQUE_INDEX_BY_SORT); and let the engine ignore it, if it wishes to do so. as a small bonus, you won't need to move alter_flags variable around, it can stay where it was, inside the if() block. > + > error= copy_data_between_tables(thd, table, new_table, > alter_info->create_list, ignore, > order_num, order, &copied, &deleted, > > === modified file 'storage/maria/ha_maria.cc' > --- a/storage/maria/ha_maria.cc 2012-08-14 16:59:28 +0000 > +++ b/storage/maria/ha_maria.cc 2012-09-27 21:29:26 +0000 > @@ -1633,6 +1633,9 @@ please, use the recipe from https://kb.askmonty.org/en/how-to-get-more-out-of-bzr-when-working-on-maria… it's much more difficult to review patches without sufficient context (that is, without function names on every chunk) > param->testflag|= T_REP_BY_SORT; > error= maria_repair_by_sort(param, file, fixed_name, > test(param->testflag & T_QUICK)); > + if (error && share->state.dupp_key != MARIA_MAX_KEY) > + print_keydup_error(share->state.dupp_key, > + ER(ER_DUP_ENTRY_WITH_KEY_NAME)); > } > } > else > > === modified file 'storage/myisam/ha_myisam.cc' > --- a/storage/myisam/ha_myisam.cc 2012-07-18 18:40:15 +0000 > +++ b/storage/myisam/ha_myisam.cc 2012-09-27 21:29:26 +0000 > @@ -1130,6 +1130,9 @@ > thd_proc_info(thd, "Repair by sorting"); > error = mi_repair_by_sort(&param, file, fixed_name, > test(param.testflag & T_QUICK)); > + if (error && share->state.dupp_key != MAX_KEY) > + print_keydup_error(share->state.dupp_key, > + ER(ER_DUP_ENTRY_WITH_KEY_NAME)); I don't understand it. print_keydup_error() takes the value to print from table->record[[0]. But - in the case of mi_repair_by_sort - how does the value end up here? What code puts the conflicting key value into table->record[0] ? > } > } > else > > === modified file 'storage/myisam/mi_check.c' > --- a/storage/myisam/mi_check.c 2012-04-07 13:58:46 +0000 > +++ b/storage/myisam/mi_check.c 2012-09-27 21:29:26 +0000 > @@ -2408,12 +2411,22 @@ > (my_bool) (!(param->testflag & T_VERBOSE)), > param->sort_buffer_length)) > { > + > + if (sort_param.sort_info->dupp) > + { > + share->state.dupp_key= sort_param.key; > + keydup_error= 1; > + } > + if (!(keydup_error && param->suppress_keydup_error_handling)) 1. please use param->testflag for flags, instead of a new special field 2. I don't see why are you doing it here, normally the error is issued below, as "Duplicate key for record at %10s against...." I'd expect to see any new code that deals with duplicate key errors at the same place where the old code is > + { > param->retry_repair= 1; > if (! param->error_printed) > mi_check_print_error(param, "Couldn't fix table with create_index_by_sort(). Error: %d", > my_errno); > + } > goto err; > } > + > /* No need to calculate checksum again. */ > sort_param.calc_checksum= 0; > free_root(&sort_param.wordroot, MYF(0)); > @@ -2533,18 +2546,27 @@ > } > if (got_error) > { > + if (!(keydup_error && param->suppress_keydup_error_handling)) > + { > if (! param->error_printed) > mi_check_print_error(param,"%d when fixing table",my_errno); You could simply set param->error_printed when you get dplicate key error - it'll avoid all further errors, you won't need to wrap them all in if()'s > + } > if (new_file >= 0) > { > (void) mysql_file_close(new_file, MYF(0)); > (void) mysql_file_delete(mi_key_file_datatmp, > param->temp_filename, MYF(MY_WME)); > + if (!(keydup_error && param->suppress_keydup_error_handling)) > + { Why do you need this if() ? > if (info->dfile == new_file) /* Retry with key cache */ > if (unlikely(mi_open_datafile(info, share, name, -1))) > param->retry_repair= 0; /* Safety */ > + } > } > + if (!(keydup_error && param->suppress_keydup_error_handling)) > + { > mi_mark_crashed_on_repair(info); > + } Why do you need to avoid mi_mark_crashed_on_repair() ? > if (killed_ptr(param)) > param->retry_repair= 0; /* No use to retry repair */ > } > @@ -4682,8 +4708,9 @@ > (!rows || rows >= MI_MIN_ROWS_TO_DISABLE_INDEXES)); > for (i=0 ; i < share->base.keys ; i++,key++) > { > - if (!(key->flag & (HA_NOSAME | HA_SPATIAL | HA_AUTO_KEY)) && > - ! mi_too_big_key_for_sort(key,rows) && info->s->base.auto_key != i+1) > + if (!(key->flag & (HA_SPATIAL | HA_AUTO_KEY)) && > + ! mi_too_big_key_for_sort(key,rows) && info->s->base.auto_key != i+1 && > + (!(key->flag & HA_NOSAME) || info->create_unique_index_by_sort)) Now, this directly contradicts the function name and the function comment. Could you please rename the function and update the comment? And instead of checking info->create_unique_index_by_sort I'd rather pass it down as an argument: void mi_disable_indexes(MI_INFO *info, ha_rows rows, my_bool only_non_unique) { > { > mi_clear_key_active(share->state.key_map, i); > info->update|= HA_STATE_CHANGED; > > === modified file 'storage/myisam/mi_extra.c' > --- a/storage/myisam/mi_extra.c 2012-07-31 17:29:07 +0000 > +++ b/storage/myisam/mi_extra.c 2012-09-27 21:29:26 +0000 > @@ -365,6 +365,9 @@ > case HA_EXTRA_CHANGE_KEY_TO_DUP: > mi_extra_keyflag(info, function); > break; > + case HA_EXTRA_CREATE_UNIQUE_INDEX_BY_SORT: > + info->create_unique_index_by_sort= 1; > + break; Two comments here: 1. I don't like the name "create unique index by sort", because ALTER TABLE itself does not create any indexes by sort or anything it simply tells the engine to optimize a bulk insertion. and it's myisam that does repair-by-sort. I mean, a better name would be a high-level name that does not imply myisam-specific details. something like HA_EXTRA_BULK_INSERT_DUPLICATES_UNLIKELY which tells the engine to optimize the following bulk insert for the case when unique key conflicts are unlikely to happen. (in myisam case it means to build unique indexes by sort, right) 2. I've discussed this with Monty and we both didn't like using extra() here, even to preserve the API compatibility. Monty thinks there's no need to bother, and we can change start_bulk_insert() any way we want. But to make it better for third-party engines, I'd suggest something like class handler { ... virtual void start_bulk_insert(ha_rows rows) {} virtual void start_bulk_insert(ha_rows rows, ulonglong flags) { start_bulk_insert(rows); } ... } that is, you introduce the new function, and keep the old one as a fallback. This way it stays compatible, and old engines won't need any changes. But myisam/maria will implement the new start_bulk_insert variant, with two arguments, and will recognize the flag HA_BULK_INSERT_DUPLICATES_UNLIKELY And no extra() is needed. > case HA_EXTRA_MMAP: > #ifdef HAVE_MMAP > mysql_mutex_lock(&share->intern_lock); > Regards, Sergei

1 0

[Maria-developers] MariaDB 5.3.10 release prep
by Daniel Bartholomew 12 Nov '12

12 Nov '12

Preliminary release notes and changelog pages have been created for 5.3.10. https://kb.askmonty.org/en/mariadb-5310-release-notes/ https://kb.askmonty.org/en/mariadb-5310-changelog/ Please let me know if there is anything in particular which should be mentioned in the release notes. Thanks. -- Daniel Bartholomew Google+ - http://gplus.to/dbart Twitter - http://twitter.com/daniel_bart MariaDB: An Enhanced Drop-in Replacement for MySQL Website - http://mariadb.org Twitter - http://twitter.com/mariadb Google+ - http://google.com/+mariadb Facebook - http://fb.com/MariaDB.dbms Knowledgebase - http://kb.askmonty.org Monty Program - http://montyprogram.com

1 0

[Maria-developers] MariaDB 10.0.0 Alpha Release Prep
by Daniel Bartholomew 12 Nov '12

12 Nov '12

All, The files for this release have been uploaded to the mirrors and we're just waiting until Monday morning, U.S. Eastern time, to announce the release. The preliminary release notes and changelog pages are here: - https://kb.askmonty.org/en/mariadb-1000-release-notes/ - https://kb.askmonty.org/en/mariadb-1000-changelog/ The release notes page is pretty sparse. Its content is from the list of "New already done Features" on the "What is MariaDB 10.0?" page. Please send me additions and corrections ASAP (or make the changes yourself). Thanks. -- Daniel Bartholomew Google+ - http://gplus.to/dbart Twitter - http://twitter.com/daniel_bart MariaDB: An Enhanced Drop-in Replacement for MySQL Website - http://mariadb.org Twitter - http://twitter.com/mariadb Google+ - http://google.com/+mariadb Facebook - http://fb.com/MariaDB.dbms Knowledgebase - http://kb.askmonty.org Monty Program - http://montyprogram.com

1 0