December 2021 - developers - lists.mariadb.org

Re: [Maria-developers] b3844107287: MDEV-16546 System versioning setting to allow history modification
by Sergei Golubchik 24 Aug '22

24 Aug '22

Hi, Aleksey! On Jun 08, Aleksey Midenkov wrote: > revision-id: b3844107287 (mariadb-10.5.2-424-gb3844107287) > parent(s): 27d66d644cf > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2021-03-01 17:43:34 +0300 > message: > > MDEV-16546 System versioning setting to allow history modification > > 1. force_fields_visible session variable makes system-invisible and > user-invisible fields visible on next table open. FLUSH TABLES > required for already opened tables. * system_versioning_insert_history * allows pseudocolumns ROW_START and ROW_END be specified in INSERT * doesn't affect invisible columns * FLUSH TABLES requirememnt is rather confusing, but let's have it, at least until I could suggest something better > 2. If secure_timestamp allows to modify timestamp variable then > following DML commands: INSERT, INSERT..SELECT and LOAD DATA can > specify row_start and row_end system field values. when system_versioning_insert_history=1 > 3. Cleaned up select_insert::send_data() from setting vers_write as > this parameter is now set on TABLE initialization. I don't see where it's set. This commit removes two table->vers_write= table->versioned(); and adds one from_field->table->vers_write= false; in Item_field::fix_fields. But where is vers_write set to true? Was it done in some other commit? > diff --git a/mysql-test/suite/versioning/r/insert.result b/mysql-test/suite/versioning/r/insert.result > index 2645d0184e8..00d1bb705c4 100644 > --- a/mysql-test/suite/versioning/r/insert.result > +++ b/mysql-test/suite/versioning/r/insert.result > @@ -112,3 +112,95 @@ x y > 9 9001 > drop table t1; > drop table t2; > +# > +# MDEV-16546 System versioning setting to allow history modification > +# > +# restart: --secure-timestamp=NO > +set @@session.time_zone='+00:00'; > +create table t1(x int) with system versioning; > +insert into t1(x) values (1); > +insert into t1(x, row_start, row_end) values (2, '1980-01-01 00:00:00', '1980-01-01 00:00:01'); > +ERROR 42S22: Unknown column 'row_start' in 'field list' > +set session force_fields_visible= 1; > +flush tables; > +show create table t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `x` int(11) DEFAULT NULL, > + `row_start` timestamp(6) GENERATED ALWAYS AS ROW START, > + `row_end` timestamp(6) GENERATED ALWAYS AS ROW END, shouldn't be visible here, only in insert and load. > + PERIOD FOR SYSTEM_TIME (`row_start`, `row_end`) > +) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 WITH SYSTEM VERSIONING > +insert into t1(x, row_start, row_end) values (3, '1980-01-01 00:00:00', '1980-01-01 00:00:01'); please, add tests for insert into t1 values (4) this should work, row_start/row_end must be mentioned explicitly. Also insert into t1 set x=5, row_start=..., row_end=... this should work too. And when only one row_start or row_end is mentioned as well. But update t1 set row_start=... insert into t1 (x, row_start, row_end) values (...) on duplicate key update row_start=... this should fail. While replace load data ... replace should work, but as DELETE+INSERT (I mean that DELETE in versioned tables doesn't actually delete history). If that's too difficult, then an error too. > +update t1 set x= x + 1; > +select x, check_row_ts(row_start, row_end) from t1 for system_time all order by x; ... > diff --git a/mysql-test/suite/versioning/t/insert.opt b/mysql-test/suite/versioning/t/insert.opt > new file mode 100644 > index 00000000000..a74d68957ef > --- /dev/null > +++ b/mysql-test/suite/versioning/t/insert.opt > @@ -0,0 +1 @@ > +--secure-timestamp=yes why is this? > -- source suite/versioning/common_finish.inc > diff --git a/mysql-test/suite/versioning/t/insert2.opt b/mysql-test/suite/versioning/t/insert2.opt > new file mode 100644 > index 00000000000..a74d68957ef > --- /dev/null > +++ b/mysql-test/suite/versioning/t/insert2.opt > @@ -0,0 +1 @@ > +--secure-timestamp=yes and this? > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index d23b190b2c5..93f8a8434e4 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -7604,6 +7604,20 @@ void THD::set_last_commit_gtid(rpl_gtid &gtid) > #endif > } > > +bool THD::vers_modify_sys_field() const don't call it vers_modify_sys_field, your previous name vers_modify_history was better. There may be other sys fields that will never be directly writable (like ROWID, for example). > +{ > + if (lex->sql_command != SQLCOM_INSERT && > + lex->sql_command != SQLCOM_INSERT_SELECT && > + lex->sql_command != SQLCOM_LOAD) > + return false; I don't think this is enough to reject insert ... on duplicate key update row_start=xxx > + if (opt_secure_timestamp >= SECTIME_REPL || this is fixed in a later commit, so ok > + (opt_secure_timestamp == SECTIME_SUPER && > + !(security_ctx->master_access & SUPER_ACL))) > + return false; > + return true; > +} > + > + > void > wait_for_commit::reinit() > { > diff --git a/sql/sql_class.h b/sql/sql_class.h > index 2913a4a8350..520ea1d63bf 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -820,6 +820,7 @@ typedef struct system_variables > > vers_asof_timestamp_t vers_asof_timestamp; > ulong vers_alter_history; > + my_bool force_fields_visible; system_versioning_insert_history > } SV; > > /** > @@ -5400,6 +5401,7 @@ class THD: public THD_count, /* this must be first */ > Item *sp_prepare_func_item(Item **it_addr, uint cols= 1); > bool sp_eval_expr(Field *result_field, Item **expr_item_ptr); > > + bool vers_modify_sys_field() const; vers_modify_history > }; > > > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc > index f0b84ab126f..84c2ae4ab36 100644 > --- a/sql/sql_insert.cc > +++ b/sql/sql_insert.cc > @@ -2014,7 +2014,7 @@ int write_record(THD *thd, TABLE *table, COPY_INFO *info, select_result *sink) > !table->file->referenced_by_foreign_key() && > (!table->triggers || !table->triggers->has_delete_triggers())) > { > - if (table->versioned(VERS_TRX_ID)) > + if (table->versioned(VERS_TRX_ID) && table->vers_write) does it even make sense? I mean direct insertion of transaction ids. > { > bitmap_set_bit(table->write_set, table->vers_start_field()->field_index); > table->file->column_bitmaps_signal(); > @@ -2029,7 +2029,7 @@ int write_record(THD *thd, TABLE *table, COPY_INFO *info, select_result *sink) > info->deleted++; > if (!table->file->has_transactions()) > thd->transaction->stmt.modified_non_trans_table= TRUE; > - if (table->versioned(VERS_TIMESTAMP)) > + if (table->versioned(VERS_TIMESTAMP) && table->vers_write) This (and the previous hunk) are in the DUP_REPLACE branch. There should be tests for these changes. > { > store_record(table, record[2]); > error= vers_insert_history_row(table); > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index f817f9086a9..4429308a097 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -6737,3 +6737,7 @@ static Sys_var_ulonglong Sys_max_rowid_filter_size( > SESSION_VAR(max_rowid_filter_size), CMD_LINE(REQUIRED_ARG), > VALID_RANGE(1024, (ulonglong)~(intptr)0), DEFAULT(128*1024), > BLOCK_SIZE(1)); > + > +static Sys_var_mybool Sys_force_fields_visible( > + "force_fields_visible", "Make invisible fields visible on next table open", "system_versioning_insert_history" "Allows direct inserts into ROW_START and ROW_END columns" > + SESSION_VAR(force_fields_visible), CMD_LINE(OPT_ARG), DEFAULT(FALSE)); > diff --git a/sql/table.cc b/sql/table.cc > index f4bdbdeac5a..520f3738c9e 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -4024,6 +4024,9 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, > { > if (!((*field_ptr)= share->field[i]->clone(&outparam->mem_root, outparam))) > goto err; > + if (thd->variables.force_fields_visible && > + (*field_ptr)->invisible <= INVISIBLE_SYSTEM) > + (*field_ptr)->invisible= VISIBLE; Not really, system_versioning_insert_history should not affect visibility of anything besides ROW_START and ROW_END pseudocolumns in the INSERT and LOAD DATA statements. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 5

Re: [Maria-developers] 734aee3cd3c: MDEV-27018 IF and COALESCE lose "json" property
by Sergei Golubchik 10 Jan '22

10 Jan '22

Hi, Alexander! On Dec 28, Alexander Barkov wrote: > revision-id: 734aee3cd3c (mariadb-10.5.13-33-g734aee3cd3c) > parent(s): 2776635cb98 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2021-12-27 17:54:31 +0400 > message: > > MDEV-27018 IF and COALESCE lose "json" property > > Hybrid functions (IF, COALESCE, etc) did not preserve the JSON property > from their arguments. The same problem was repeatable for single row subselects. > > The problem happened because the method Item::is_json_type() was inconsistently > implemented across the Item hierarchy. For example, Item_hybrid_func > and Item_singlerow_subselect did not override is_json_type(). Okay. The approach is fine, but see some questions below: > diff --git a/sql/field.cc b/sql/field.cc > index 2c768527ced..8fa3bbd538c 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -7277,6 +7277,19 @@ bool Field_longstr::send(Protocol *protocol) > } > > > +const Type_handler *Field_string::type_handler() const > +{ > + if (is_var_string()) > + return &type_handler_var_string; > + /* > + This is a temporary solution and will be fixed soon (in 10.9?). > + Type_handler_string_json will provide its own Field_string_json. why? > + */ > + if (Type_handler_json_common::has_json_valid_constraint(this)) could you please use names that don't depend on how exactly we detect json? especially if the goal is to move to FORMAT JSON syntax > + return &type_handler_string_json; > + return &type_handler_string; > +} > + > /* Copy a string and fill with space */ > > int Field_string::store(const char *from, size_t length,CHARSET_INFO *cs) > diff --git a/sql/sql_type.cc b/sql/sql_type.cc > index c1801c1ae3e..3b2753d80a6 100644 > --- a/sql/sql_type.cc > +++ b/sql/sql_type.cc > @@ -1755,19 +1755,110 @@ const Type_handler *Type_handler_typelib::cast_to_int_type_handler() const > > /***************************************************************************/ > > +class Recursive_type_pair_iterator > +{ > + const Type_handler *m_a; > + const Type_handler *m_b; > + uint m_switched_to_base_count; > +public: > + Recursive_type_pair_iterator(const Type_handler *a, > + const Type_handler *b, > + uint switched_to_base_count= 0) > + :m_a(a), m_b(b), m_switched_to_base_count(switched_to_base_count) > + { } > + const Type_handler *a() const { return m_a; } > + const Type_handler *b() const { return m_b; } > + Recursive_type_pair_iterator base() const > + { > + Recursive_type_pair_iterator res(m_a->type_handler_base(), > + m_b->type_handler_base()); > + res.m_switched_to_base_count= (res.m_a != NULL) + (res.m_b != NULL); > + if (res.m_a == NULL) > + res.m_a= m_a; > + if (res.m_b == NULL) > + res.m_b= m_b; > + return res; > + } > + bool done() const > + { > + switch (m_switched_to_base_count) > + { > + case 2: > + /* > + Expression: > + COALESCE(TYPE1, TYPE2) > + > + where both type handlers derive from some other handlers, e.g. > + VARCHAR -> TYPE1 > + TEXT -> TYPE2 > + > + Continue aggregation as: > + SELECT COALESCE(VARCHAR, TEXT) > + > + Note, we now have only one level of data type inheritance, > + e.g. VARCHAR -> VARCHAR/JSON > + > + This code branch will change when we have longer inheritance chains: > + A0 -> A1 -> A2 -> A3 > + B0 -> B1 -> B2 > + > + Functions like COALESE(A2, B2) will need to do full overload resolution: > + - iterate through all pairs from the below matrix > + - find all existing candidates > + - resolve ambiguities in case multiple candidates, etc. and I don't see how you're going to do that. you need to try m_a+m_b.base(), m_a.base()+m_b, and m_a.base()+m_b.base(). And that's not what you're doing. I suggest to add an assert that m_switched_to_base_count < 2 and stop trying to solve complex cases until we have at least one. > + > + A0 A1 A2 A3 > + B0 . . . . > + B1 . . . . > + B2 . . . . > + */ > + return false; > + > + case 1: > + /* > + Expression: > + COALESCE(TYPE1, TEXT) > + > + If only one handler derives from some other handler: > + VARCHAR-> TYPE1 > + > + Continue aggregation as: > + SELECT COALESCE(VARCHAR, TEXT) > + */ > + return false; > + > + case 0: > + default: > + /* > + Non of the two handlers are derived from other handlers. > + Nothing left to try. > + */ > + return true; > + } > + } > +}; > diff --git a/sql/sql_type_json.h b/sql/sql_type_json.h > index 6c4ee8cb2eb..4a394809a06 100644 > --- a/sql/sql_type_json.h > +++ b/sql/sql_type_json.h > @@ -21,18 +21,145 @@ ... > + bool Item_append_extended_type_info(Send_field_extended_metadata *to, > + const Item *item) const > + { > + return set_format_name(to); // Send "format=json" in the protocol > + } > + > + bool Item_hybrid_func_fix_attributes(THD *thd, > + const char *name, > + Type_handler_hybrid_field_type *hybrid, > + Type_all_attributes *attr, > + Item **items, uint nitems) > + const override > + { > + if (BASE::Item_hybrid_func_fix_attributes(thd, name, hybrid, attr, > + items, nitems)) > + return true; > + /* > + The above call can change the type handler on "hybrid", e.g. > + choose a proper BLOB type handler according to the calculated max_length. > + Convert general purpose string type handler to its JSON counterpart. > + This makes hybrid functions preserve JSON data types, e.g.: > + COALESCE(json_expr1, json_expr2) -> JSON > + */ > + hybrid->set_handler(json_type_handler_from_generic(hybrid->type_handler())); I don't get it. It's supposed to "fix attributes", like signed/unsigned, max length, etc. The correct type handler should've been set already, otherwise this type handler method wouldn't even be called. > + return false; > + } > }; > > + > +class Type_handler_string_json: > + public Type_handler_general_purpose_string_to_json<Type_handler_string, > + type_handler_string> > +{ }; > diff --git a/sql/sql_type_json.cc b/sql/sql_type_json.cc > index a804366ec03..a84f720bcc9 100644 > --- a/sql/sql_type_json.cc > +++ b/sql/sql_type_json.cc > @@ -20,17 +20,111 @@ > #include "sql_class.h" > > > -Type_handler_json_longtext type_handler_json_longtext; > +Named_type_handler<Type_handler_string_json> > + type_handler_string_json("char/json"); do you mean these names are not shown anywhere? > +Named_type_handler<Type_handler_varchar_json> > + type_handler_varchar_json("varchar/json"); > + > +Named_type_handler<Type_handler_tiny_blob_json> > + type_handler_tiny_blob_json("tinyblob/json"); > + > +Named_type_handler<Type_handler_blob_json> > + type_handler_blob_json("blob/json"); > + > +Named_type_handler<Type_handler_medium_blob_json> > + type_handler_medium_blob_json("mediumblob/json"); > + > +Named_type_handler<Type_handler_long_blob_json> > + type_handler_long_blob_json("longblob/json"); ... > +/* > + This method ressembles what Field_blob::type_handler() resembles (also below) > + does for general purpose BLOB type handlers. > +*/ > +const Type_handler * > +Type_handler_json_common::json_blob_type_handler_by_length_bytes(uint len) > +{ > + switch (len) { > + case 1: return &type_handler_tiny_blob_json; > + case 2: return &type_handler_blob_json; > + case 3: return &type_handler_medium_blob_json; > + } > + return &type_handler_long_blob_json; > +} Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] 8a84f5a40c1: MDEV-24176 Preparations
by Sergei Golubchik 06 Jan '22

06 Jan '22

Hi, Aleksey! On Oct 17, Aleksey Midenkov wrote: > commit 8a84f5a40c1 > Author: Aleksey Midenkov <midenok(a)gmail.com> > Date: Thu May 27 17:00:14 2021 +0300 > > MDEV-24176 Preparations > > 1. moved fix_vcol_exprs() call to open_table() > > mysql_alter_table() doesn't do lock_tables() so it cannot win from > fix_vcol_exprs() from there. Tests affected: main.default_session This is likely wrong, but the old code was wrong too. Neither open_table nor lock_tables is called under LOCK TABLES, but the session environment can change, I suspect. > > 2. cleanup_excluding_fields_processor removed. > > That was just a quick hack to exclude wrongly working Item_field from > processing. Now it works due to correct execution environment (see > next commit). Related to MDEV-10355 does it work now, in that commit, or only after the next commit? what exactly do you mean by correct execution environment? > > 3. Vanilla cleanups and comments. > > diff --git a/sql/item.h b/sql/item.h > index cc1914a7ad4..7b7fe04f0b2 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -2586,6 +2585,12 @@ class Item_ident :public Item_result_field > const char *db_name; > const char *table_name; > const char *field_name; > + /* > + NOTE: came from TABLE::alias_name_used and this is only a hint! It works > + only in need_correct_ident() condition. On other cases it is FALSE even if > + table_name is alias! It cannot be TRUE in these cases, lots of spaghetti > + logic depends on that. could you elaborate on that? > + */ > bool alias_name_used; /* true if item was resolved against alias */ > /* > Cached value of index for this field in table->field array, used by prep. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 5

Re: [Maria-developers] 803b977229c: MDEV-26698: Incorrect row number upon INSERT .. SELECT from the same table: rows are counted twice
by Sergei Golubchik 31 Dec '21

31 Dec '21

Hi, Rucha! Ok to push. One suggestion below. On Dec 31, Rucha Deodhar wrote: > revision-id: 803b977229c (mariadb-10.2.40-192-g803b977229c) > parent(s): 42fea34d4a9 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-12-29 16:09:10 +0530 > message: > > MDEV-26698: Incorrect row number upon INSERT .. SELECT from the same > table: rows are counted twice > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index a331f4f3dbc..c8d5170f568 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -18972,7 +18972,7 @@ evaluate_join_record(JOIN *join, JOIN_TAB *join_tab, > bool shortcut_for_distinct= join_tab->shortcut_for_distinct; > ha_rows found_records=join->found_records; > COND *select_cond= join_tab->select_cond; > - bool select_cond_result= TRUE; > + bool select_cond_result= TRUE, unlock_row= TRUE; > > DBUG_ENTER("evaluate_join_record"); > DBUG_PRINT("enter", > @@ -19124,6 +19124,7 @@ evaluate_join_record(JOIN *join, JOIN_TAB *join_tab, > > if (found) > { > + unlock_row= false; > enum enum_nested_loop_state rc; > /* A match from join_tab is found for the current partial join. */ > rc= (*join_tab->next_select)(join, join_tab+1, 0); > @@ -19147,11 +19148,6 @@ evaluate_join_record(JOIN *join, JOIN_TAB *join_tab, > if (shortcut_for_distinct && found_records != join->found_records) > DBUG_RETURN(NESTED_LOOP_NO_MORE_ROWS); instead of unlock_row() you can put DBUG_RETURN(NESTED_LOOP_OK); here. Might be a bit simpler to read. > } > - else > - { > - join->thd->get_stmt_da()->inc_current_row_for_warning(); > - join_tab->read_record.unlock_row(join_tab); > - } > } > else > { > @@ -19160,6 +19156,9 @@ evaluate_join_record(JOIN *join, JOIN_TAB *join_tab, > with the beginning coinciding with the current partial join. > */ > join->join_examined_rows++; > + } > + if (unlock_row) > + { > join->thd->get_stmt_da()->inc_current_row_for_warning(); > join_tab->read_record.unlock_row(join_tab); > } > @@ -26945,6 +26944,12 @@ AGGR_OP::end_send() > table->reginfo.lock_type= TL_UNLOCK; > > bool in_first_read= true; > + > + /* > + Reset the counter before copying rows from internal temporary table to > + INSERT table. > + */ > + join_tab->join->thd->get_stmt_da()->reset_current_row_for_warning(); > while (rc == NESTED_LOOP_OK) > { > int error; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 4774601a3cd: MDEV-26698: Incorrect row number upon INSERT .. SELECT from the same table: rows are counted twice
by Sergei Golubchik 31 Dec '21

31 Dec '21

Hi, Rucha! On Dec 29, Rucha Deodhar wrote: > On Wed, Dec 29, 2021 at 12:50 AM Sergei Golubchik <serg(a)mariadb.org> wrote: > > > Why did put your fix here in a common code path in select, and not > > in, say, select_insert::send_data() ? > > Yes, the counter can be incremented in select_insert::send_data() too. > > But after writing record to INSERT table, we go back to > evaluate_join_records() where the counter is incremented > unconditionally. And CREATE TABLE...SELECT was failing with wrong > output because it also uses this code. > > I could have incremented counter conditionally so that it doesn't > increment twice like so: > > increment counter in select_insert::send_data() after record is written > if (thd->lex->sql_command == SQLCOM_INSERT_SELECT || > thd->lex->sql_command == SQLCOM_CREATE_TABLE) I'd think you wouldn't need this if(), as select_insert::send_data() is not used for other sql commands. > And increment counter in evalaute_join_records() after one nested loop > iteration ends (end_send()) > if (join->thd->lex->sql_command != SQLCOM_INSERT_SELECT && > join->thd->lex->sql_command != SQLCOM_CREATE_TABLE) I see. That's not good. > I had initially implemented it this way but it kinda looked hacky to me. > So I did a more general solution. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 4774601a3cd: MDEV-26698: Incorrect row number upon INSERT .. SELECT from the same table: rows are counted twice
by Sergei Golubchik 28 Dec '21

28 Dec '21

Hi, Rucha! Note test failures in buildbot, don't forget to update all result files before pushing. On Dec 28, Rucha Deodhar wrote: > revision-id: 4774601a3cd (mariadb-10.2.40-192-g4774601a3cd) > parent(s): 42fea34d4a9 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-12-27 16:48:04 +0530 > message: > > MDEV-26698: Incorrect row number upon INSERT .. SELECT from the same > table: rows are counted twice > > Analysis: When the table we are trying to insert into and the SELECT table > are same for INSERT ... SELECT, rows from the SELECT table are copied into > internal temporary table and then to the INSERT table. We only want to > count the rows when we start inserting into the table. > Fix: Reset the counter to 1 before starting to copy from internal temporary > table to select table and then increment the counter. > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index a331f4f3dbc..7a50f3d74ac 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -3832,6 +3832,8 @@ mysql_select(THD *thd, > } > } > > + thd->get_stmt_da()->reset_current_row_for_warning(); What is this for? It wasn't needed before. > if ((err= join->optimize())) > { > goto err; // 1 > @@ -19124,10 +19126,10 @@ evaluate_join_record(JOIN *join, JOIN_TAB *join_tab, > > if (found) > { > + unlock_row= false; > enum enum_nested_loop_state rc; > /* A match from join_tab is found for the current partial join. */ > rc= (*join_tab->next_select)(join, join_tab+1, 0); > - join->thd->get_stmt_da()->inc_current_row_for_warning(); By moving this down you skip few returns, incl NESTED_LOOP_OK and NESTED_LOOP_NO_MORE_ROWS. Are you sure it's fine? Won't the counter be off, if the query execution continues after, say, NESTED_LOOP_OK? > if (rc != NESTED_LOOP_OK && rc != NESTED_LOOP_NO_MORE_ROWS) > DBUG_RETURN(rc); > if (return_tab < join->return_tab) > @@ -26945,6 +26943,12 @@ AGGR_OP::end_send() > table->reginfo.lock_type= TL_UNLOCK; > > bool in_first_read= true; > + > + /* > + Reset the counter before copying rows from internal temporary table to > + INSERT table. this code is also used for group by. Please, add a test for insert ... select ... group by Why did put your fix here in a common code path in select, and not in, say, select_insert::send_data() ? > + */ > + join_tab->join->thd->get_stmt_da()->reset_current_row_for_warning(); > while (rc == NESTED_LOOP_OK) > { > int error; > @@ -26968,6 +26972,8 @@ AGGR_OP::end_send() > else > { > rc= evaluate_join_record(join, join_tab, 0); > + /* Increment the counter after copying rows. */ > + join_tab->join->thd->get_stmt_da()->inc_current_row_for_warning(); > } > } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] MDEV-26996: On removing SEL_ARG::is_ascending
by Sergey Petrunia 20 Dec '21

20 Dec '21

Hello Igor, Please check out this: https://github.com/MariaDB/server/tree/bb-10.8-mdev26996-no-sel-arg-ascendi… This is a patch that removes SEL_ARG::is_ascending. Does it make the code better in your opinion? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

Re: [Maria-developers] [Commits] 677643a: MDEV-27262 Unexpected index intersection with full index scan for an index
by Sergey Petrunia 20 Dec '21

20 Dec '21

Hello Igor, I'm trying examples with this patch: create table t1 (a int not null, key(a)); insert into t1 select seq from seq_1_to_1000; First, I debug this example: explain select * from t1 where a <10 or a>=10; The execution enters key_or(), passes through the new code this patch has added and key_or() returns NULL. Good. But if I modify the example slightly: explain select * from t1 where a <20 or a>=10 ; Here, key_or() still returns a SEL_ARG object that represents an infinite range: (gdb) fini Run till exit from #0 key_or (...) 0x0000555555f1fca2 in tree_or (...) Value returned is $24 = (SEL_ARG *) 0x7fff70064d88 (gdb) p dbug_print_sel_arg($24) $26 = 0x7fff70024390 "SEL_ARG(-inf<=a<=+inf)" Should this case be fixed as well? On Fri, Dec 17, 2021 at 02:11:40PM -0800, IgorBabaev wrote: > revision-id: 677643a80986107491b7886441f2828384f0494b (mariadb-10.2.31-1286-g677643a) > parent(s): 8bb55633699612279744c055e22eeca8d4058273 > author: Igor Babaev > committer: Igor Babaev > timestamp: 2021-12-17 14:11:39 -0800 > message: > > MDEV-27262 Unexpected index intersection with full index scan for an index > > If when extracting a range condition foran index from the WHERE condition I think the comment wording could be improved also. > Range Optimizer sees that the range condition covers the whole index then > such condition should be discarded because cannot it be used in any range > scan. In some cases Range Optimizer really does it, but there remained some > conditions for which it was not done. As a result the optimizer could > produce index merge plans with the full index scan for one of the indexes > participating in the index merge. > This could be observed in one of the test cases from index_merge1.inc > where a plan with index_merge_sort_union was produced and in the test case > reported for this bug where a plan with index_merge_sort_intersect was > produced. In both cases one of two index scans participating in index merge > ran over the whole index. > The patch slightly changes the original above mentioned test case from > index_merge1.inc to be able to produce an intended plan employing > index_merge_sort_union. The original query was left to show that index > merge is not used for it anymore. > It should be noted that for the plan with index_merge_sort_intersect could > be chosen for execution only due to a defect in the InnoDB code that > returns wrong estimates for the cardinality of big ranges. > > This bug led to serious problems in 10.4+ where the optimization using > Rowid filters is employed (see mdev-26446). > > Approved by Oleksandr Byelkin <sanja(a)mariadb.com> > > --- > mysql-test/include/index_merge1.inc | 8 +++- > mysql-test/r/index_merge_myisam.result | 12 +++-- > mysql-test/r/range_innodb.result | 81 ++++++++++++++++++++++++++++++++++ > mysql-test/t/range_innodb.test | 78 ++++++++++++++++++++++++++++++++ > sql/opt_range.cc | 7 +++ > 5 files changed, 181 insertions(+), 5 deletions(-) > > diff --git a/mysql-test/include/index_merge1.inc b/mysql-test/include/index_merge1.inc > index b168a76..440f1f7 100644 > --- a/mysql-test/include/index_merge1.inc > +++ b/mysql-test/include/index_merge1.inc > @@ -150,15 +150,19 @@ explain select * from t0 where > (((key3 <7 and key7 < 6) or key5 < 2) and (key5 < 5 or key6 < 6)); > > explain select * from t0 where > - ((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4)) > + ((key3 < 4 or key5 < 4) and (key1 < 4 or key2 < 4)) > or > ((key3 >=5 or key5 < 2) and (key5 < 5 or key6 < 6)); > > explain select * from t0 force index(i1, i2, i3, i4, i5, i6 ) where > - ((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4)) > + ((key3 < 4 or key5 < 4) and (key1 < 4 or key2 < 4)) > or > ((key3 >=5 or key5 < 2) and (key5 < 5 or key6 < 6)); > > +explain select * from t0 force index(i1, i2, i3, i4, i5, i6 ) where > + ((key3 < 5 or key5 < 4) and (key1 < 4 or key2 < 4)) > + or > + ((key3 >=5 or key5 < 2) and (key5 < 5 or key6 < 6)); > # 8. Verify that "order by" after index merge uses filesort > select * from t0 where key1 < 5 or key8 < 4 order by key1; > > diff --git a/mysql-test/r/index_merge_myisam.result b/mysql-test/r/index_merge_myisam.result > index 5a23092..a096c34 100644 > --- a/mysql-test/r/index_merge_myisam.result > +++ b/mysql-test/r/index_merge_myisam.result > @@ -173,17 +173,23 @@ or > id select_type table type possible_keys key key_len ref rows Extra > 1 SIMPLE t0 index_merge i1,i2,i3,i5,i6,i7 i3,i5 4,4 NULL 11 Using sort_union(i3,i5); Using where > explain select * from t0 where > -((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4)) > +((key3 < 4 or key5 < 4) and (key1 < 4 or key2 < 4)) > or > ((key3 >=5 or key5 < 2) and (key5 < 5 or key6 < 6)); > id select_type table type possible_keys key key_len ref rows Extra > 1 SIMPLE t0 ALL i1,i2,i3,i5,i6 NULL NULL NULL 1024 Using where > explain select * from t0 force index(i1, i2, i3, i4, i5, i6 ) where > -((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4)) > +((key3 < 4 or key5 < 4) and (key1 < 4 or key2 < 4)) > or > ((key3 >=5 or key5 < 2) and (key5 < 5 or key6 < 6)); > id select_type table type possible_keys key key_len ref rows Extra > -1 SIMPLE t0 index_merge i1,i2,i3,i5,i6 i3,i5 0,4 NULL 1024 Using sort_union(i3,i5); Using where > +1 SIMPLE t0 index_merge i1,i2,i3,i5,i6 i3,i5 4,4 NULL 1024 Using sort_union(i3,i5); Using where > +explain select * from t0 force index(i1, i2, i3, i4, i5, i6 ) where > +((key3 < 5 or key5 < 4) and (key1 < 4 or key2 < 4)) > +or > +((key3 >=5 or key5 < 2) and (key5 < 5 or key6 < 6)); > +id select_type table type possible_keys key key_len ref rows Extra > +1 SIMPLE t0 ALL i1,i2,i3,i5,i6 NULL NULL NULL 1024 Using where > select * from t0 where key1 < 5 or key8 < 4 order by key1; > key1 key2 key3 key4 key5 key6 key7 key8 > 1 1 1 1 1 1 1 1023 > diff --git a/mysql-test/r/range_innodb.result b/mysql-test/r/range_innodb.result > index f2349f2..ccb6da3 100644 > --- a/mysql-test/r/range_innodb.result > +++ b/mysql-test/r/range_innodb.result > @@ -108,3 +108,84 @@ DROP TABLE t0,t1; > SET @@GLOBAL.debug_dbug = @saved_dbug; > set @@optimizer_switch= @optimizer_switch_save; > # End of 10.1 tests > +# > +# MDEV-27262: Index intersection with full scan over an index > +# > +CREATE TABLE t1 ( > +id int(10) unsigned NOT NULL AUTO_INCREMENT, > +p char(32) DEFAULT NULL, > +es tinyint(3) unsigned NOT NULL DEFAULT 0, > +er tinyint(3) unsigned NOT NULL DEFAULT 0, > +x mediumint(8) unsigned NOT NULL DEFAULT 0, > +PRIMARY KEY (id), > +INDEX es (es), > +INDEX x (x), > +INDEX er (er,x), > +INDEX p (p) > +) ENGINE=InnoDB DEFAULT CHARSET=latin1; > +insert into t1(es,er) select 0, 1 from seq_1_to_45; > +insert into t1(es,er) select 0, 2 from seq_1_to_49; > +insert into t1(es,er) select 0, 3 from seq_1_to_951; > +insert into t1(es,er) select 0, 3 from seq_1_to_1054; > +insert into t1(es,er) select 0, 6 from seq_1_to_25; > +insert into t1(es,er) select 0, 11 from seq_1_to_1; > +insert into t1(es,er) select 1, 1 from seq_1_to_45; > +insert into t1(es,er) select 1, 2 from seq_1_to_16; > +insert into t1(es,er) select 1, 3 from seq_1_to_511; > +insert into t1(es,er) select 1, 4 from seq_1_to_687; > +insert into t1(es,er) select 1, 6 from seq_1_to_50; > +insert into t1(es,er) select 1, 7 from seq_1_to_4; > +insert into t1(es,er) select 1, 11 from seq_1_to_1; > +insert into t1(es,er) select 2, 1 from seq_1_to_82; > +insert into t1(es,er) select 2, 2 from seq_1_to_82; > +insert into t1(es,er) select 2, 3 from seq_1_to_1626; > +insert into t1(es,er) select 2, 4 from seq_1_to_977; > +insert into t1(es,er) select 2, 6 from seq_1_to_33; > +insert into t1(es,er) select 2, 11 from seq_1_to_1; > +insert into t1(es,er) select 3, 1 from seq_1_to_245; > +insert into t1(es,er) select 3, 2 from seq_1_to_81; > +insert into t1(es,er) select 3, 3 from seq_1_to_852; > +insert into t1(es,er) select 3, 4 from seq_1_to_2243; > +insert into t1(es,er) select 3, 6 from seq_1_to_44; > +insert into t1(es,er) select 3, 11 from seq_1_to_1; > +insert into t1(es,er) select 4, 1 from seq_1_to_91; > +insert into t1(es,er) select 4, 2 from seq_1_to_83; > +insert into t1(es,er) select 4, 3 from seq_1_to_297; > +insert into t1(es,er) select 4, 4 from seq_1_to_2456; > +insert into t1(es,er) select 4, 6 from seq_1_to_19; > +insert into t1(es,er) select 4, 11 from seq_1_to_1; > +update t1 set p='foobar'; > +update t1 set x=0; > +set @save_isp=@@innodb_stats_persistent; > +set global innodb_stats_persistent= 1; > +analyze table t1; > +Table Op Msg_type Msg_text > +test.t1 analyze status OK > +set optimizer_switch='index_merge_sort_intersection=on'; > +SELECT * FROM t1 > +WHERE ((p = 'foo' AND er != 4) OR er = 4 ) AND (es >= 4) LIMIT 2; > +id p es er x > +14645 foobar 4 4 0 > +14646 foobar 4 4 0 > +EXPLAIN EXTENDED SELECT * FROM t1 > +WHERE ((p = 'foo' AND er != 4) OR er = 4 ) AND (es >= 4) LIMIT 2; > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t1 range es,er,p es 1 NULL # 100.00 Using index condition; Using where > +Warnings: > +Note 1003 select `test`.`t1`.`id` AS `id`,`test`.`t1`.`p` AS `p`,`test`.`t1`.`es` AS `es`,`test`.`t1`.`er` AS `er`,`test`.`t1`.`x` AS `x` from `test`.`t1` where (`test`.`t1`.`p` = 'foo' and `test`.`t1`.`er` <> 4 or `test`.`t1`.`er` = 4) and `test`.`t1`.`es` >= 4 limit 2 > +set optimizer_switch='index_merge_sort_intersection=off'; > +SELECT * FROM t1 > +WHERE ((p = 'foo' AND er != 4) OR er = 4 ) AND (es >= 4) LIMIT 2; > +id p es er x > +14645 foobar 4 4 0 > +14646 foobar 4 4 0 > +EXPLAIN EXTENDED SELECT * FROM t1 > +WHERE ((p = 'foo' AND er != 4) OR er = 4 ) AND (es >= 4) LIMIT 2; > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t1 range es,er,p es 1 NULL # 100.00 Using index condition; Using where > +Warnings: > +Note 1003 select `test`.`t1`.`id` AS `id`,`test`.`t1`.`p` AS `p`,`test`.`t1`.`es` AS `es`,`test`.`t1`.`er` AS `er`,`test`.`t1`.`x` AS `x` from `test`.`t1` where (`test`.`t1`.`p` = 'foo' and `test`.`t1`.`er` <> 4 or `test`.`t1`.`er` = 4) and `test`.`t1`.`es` >= 4 limit 2 > +set optimizer_switch='index_merge_sort_intersection=default'; > +set global innodb_stats_persistent= @save_isp; > +DROP TABLE t1; > +# End of 10.2 tests > diff --git a/mysql-test/t/range_innodb.test b/mysql-test/t/range_innodb.test > index 428e5c2..7420e72 100644 > --- a/mysql-test/t/range_innodb.test > +++ b/mysql-test/t/range_innodb.test > @@ -116,3 +116,81 @@ SET @@GLOBAL.debug_dbug = @saved_dbug; > set @@optimizer_switch= @optimizer_switch_save; > > --echo # End of 10.1 tests > + > +--echo # > +--echo # MDEV-27262: Index intersection with full scan over an index > +--echo # > + > +--source include/have_sequence.inc > + > +CREATE TABLE t1 ( > + id int(10) unsigned NOT NULL AUTO_INCREMENT, > + p char(32) DEFAULT NULL, > + es tinyint(3) unsigned NOT NULL DEFAULT 0, > + er tinyint(3) unsigned NOT NULL DEFAULT 0, > + x mediumint(8) unsigned NOT NULL DEFAULT 0, > + PRIMARY KEY (id), > + INDEX es (es), > + INDEX x (x), > + INDEX er (er,x), > + INDEX p (p) > +) ENGINE=InnoDB DEFAULT CHARSET=latin1; > + > +insert into t1(es,er) select 0, 1 from seq_1_to_45; > +insert into t1(es,er) select 0, 2 from seq_1_to_49; > +insert into t1(es,er) select 0, 3 from seq_1_to_951; > +insert into t1(es,er) select 0, 3 from seq_1_to_1054; > +insert into t1(es,er) select 0, 6 from seq_1_to_25; > +insert into t1(es,er) select 0, 11 from seq_1_to_1; > +insert into t1(es,er) select 1, 1 from seq_1_to_45; > +insert into t1(es,er) select 1, 2 from seq_1_to_16; > +insert into t1(es,er) select 1, 3 from seq_1_to_511; > +insert into t1(es,er) select 1, 4 from seq_1_to_687; > +insert into t1(es,er) select 1, 6 from seq_1_to_50; > +insert into t1(es,er) select 1, 7 from seq_1_to_4; > +insert into t1(es,er) select 1, 11 from seq_1_to_1; > +insert into t1(es,er) select 2, 1 from seq_1_to_82; > +insert into t1(es,er) select 2, 2 from seq_1_to_82; > +insert into t1(es,er) select 2, 3 from seq_1_to_1626; > +insert into t1(es,er) select 2, 4 from seq_1_to_977; > +insert into t1(es,er) select 2, 6 from seq_1_to_33; > +insert into t1(es,er) select 2, 11 from seq_1_to_1; > +insert into t1(es,er) select 3, 1 from seq_1_to_245; > +insert into t1(es,er) select 3, 2 from seq_1_to_81; > +insert into t1(es,er) select 3, 3 from seq_1_to_852; > +insert into t1(es,er) select 3, 4 from seq_1_to_2243; > +insert into t1(es,er) select 3, 6 from seq_1_to_44; > +insert into t1(es,er) select 3, 11 from seq_1_to_1; > +insert into t1(es,er) select 4, 1 from seq_1_to_91; > +insert into t1(es,er) select 4, 2 from seq_1_to_83; > +insert into t1(es,er) select 4, 3 from seq_1_to_297; > +insert into t1(es,er) select 4, 4 from seq_1_to_2456; > +insert into t1(es,er) select 4, 6 from seq_1_to_19; > +insert into t1(es,er) select 4, 11 from seq_1_to_1; > +update t1 set p='foobar'; > +update t1 set x=0; > +set @save_isp=@@innodb_stats_persistent; > +set global innodb_stats_persistent= 1; > +analyze table t1; > + > +let $q= > +SELECT * FROM t1 > + WHERE ((p = 'foo' AND er != 4) OR er = 4 ) AND (es >= 4) LIMIT 2; > + > +set optimizer_switch='index_merge_sort_intersection=on'; > +eval $q; > +--replace_column 9 # > +eval EXPLAIN EXTENDED $q; > + > +set optimizer_switch='index_merge_sort_intersection=off'; > +# execution of $q and explain for it led to an assertion failure in 10.4 > +# (with the optimizer switch rowid_filter set to 'on') > +eval $q; > +--replace_column 9 # > +eval EXPLAIN EXTENDED $q; > +set optimizer_switch='index_merge_sort_intersection=default'; > + > +set global innodb_stats_persistent= @save_isp; > +DROP TABLE t1; > + > +--echo # End of 10.2 tests > diff --git a/sql/opt_range.cc b/sql/opt_range.cc > index f3f1843..2e05b88 100644 > --- a/sql/opt_range.cc > +++ b/sql/opt_range.cc > @@ -9413,6 +9413,13 @@ key_or(RANGE_OPT_PARAM *param, SEL_ARG *key1,SEL_ARG *key2) > key2->copy_min(tmp); > if (!(key1=key1->tree_delete(tmp))) > { // Only one key in tree > + if (key2->min_flag & NO_MIN_RANGE && > + key2->max_flag & NO_MAX_RANGE) > + { > + if (key2->maybe_flag) > + return new SEL_ARG(SEL_ARG::MAYBE_KEY); > + return 0; // Always true OR > + } > key1=key2; > key1->make_root(); > key2=key2_next; BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

[Maria-developers] Review: bb-10.8-wlad-MDEV-26713
by Alexander Barkov 13 Dec '21

13 Dec '21

Hi Wlad, You've done a great piece of work. Thanks and congrats. I generally like all the changes. Have a few comments only, please see below. commit 1411d5f40ceaf2eecabbcf68588743439003f4a2 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 19 11:42:46 2021 +0100 Windows : incorrect handling of non-ASCIIs in get_tty_password Why no MDEV? + wchar_t wbuf[80]; + char to[80*3]; /* there is at most 3 bytes per wchar, in any codepage */ + wchar_t *pos=wbuf,*end=wbuf + array_elements(wbuf)-1; Isn't it "at most 4 bytes per char"? ############################################################# commit 7af691ca11c95a385aff4889a56870d70843e78d Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 19 12:03:48 2021 +0100 MDEV-27090: Windows client - ReadConsoleA does not work correctly with UTF8 codepage Corresponding Windows bug https://github.com/microsoft/terminal/issues/4551 Use ReadConsoleW instead and convert to console's input codepage, to workaround. Also, disable VT sequences in the console output, as we do not knows what type of data comes with SELECT, we do not want VT escapes there. Remove my_cgets() + while (nchars > 0) + { + if (wstrbuf[nchars - 1] != '\n' && wstrbuf[nchars - 1] != '\r') + break; + wstrbuf[--nchars]= 0; + } + + wstrbuf[nchars]= 0; The inner assignment wstrbuf[--nchars]=0 inside every iteration is not needed. I'd write it this way: for ( ; nchars > 0 ; nchars--) { if (wstrbuf[nchars - 1] != '\n' && wstrbuf[nchars - 1] != '\r') break; } wstrbuf[nchars]= 0; But the outer assignment to 0 does not seem to be needed either. Why do it? my_convert() does not need the trailing zero. It uses only sizes passed. So this should be enough: + strbuf[len]= 0; - pthread_auto_mutex_decl(my_conio_cs); - - /* lock the console for the current process*/ - if (pthread_auto_mutex_lock(my_conio_cs, GetCurrentProcessId(), INFINITE)) - { There was a mutex lock inside my_cgets(). Why was it needed before the patch and why is it not needed any more? ##################################################### commit 5c09e9ce4f922fe9674f3f85329742c0c5498a28 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 19 14:03:51 2021 +0100 MDEV-27092 Windows - services that have non-ASCII characters do not work with activeCodePage=UTF8 CreateServiceA, OpenServiceA, and couple of other functions do not work correctly with non-ASCII character, in the special case where application has defined activeCodePage=UTF8. ... - Copyright (c) 2011, 2012, Monty Program Ab + Copyright (c) 2011, 2021 Monty Program Ab MariaDB Corporation? This code: + AWSTRDUP(w_ServiceName,lpServiceName); + AWSTRDUP(w_DisplayName,lpDisplayName); + AWSTRDUP(w_BinaryPathName, lpBinaryPathName); + AWSTRDUP(w_LoadOrderGroup, lpLoadOrderGroup); + AWSTRDUP(w_Dependencies, lpDependencies); + AWSTRDUP(w_ServiceStartName, lpServiceStartName); + AWSTRDUP(w_Password, lpPassword); + + free(w_ServiceName); + free(w_DisplayName); + free(w_BinaryPathName); + free(w_LoadOrderGroup); + free(w_Dependencies); + free(w_ServiceStartName); + free(w_Password); duplicates two times. Consider wrapping these variables into a class and reuse the class in both cases. ########################################################## commit d4056a28ec5ae0c9a0c3144a8e90357dc574723f Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 19 14:14:38 2021 +0100 MDEV-27093 Do not pass root password in clear text from mariadb-install-db.exe to bootstrap Pass the hash instead, so that bootstrap would not misinterpret it for string with different charset. The password was passed using hex encoding. But the comment says it was clear text. Wrong comment? I did not understand the change. Reading the MDEV did not make it clearer. What was wrong? How does the patch fix the problem? Can we discuss on slack? ######################################################## commit fa29a64de918d2045df3cce1d4a546de5597c0bd Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Mon Nov 22 12:29:15 2021 +0100 MDEV-26713 set activeCodePage=UTF8 for windows programs - Use corresponding entry in the manifest, as described in https://docs.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-c… - Set console codepage(s) to UTF8, if ANSI codepage is UTF8 This effectively causes UTF8 to be used as default client encoding, for Windows 1903 and later - Allow UTF8 on the command line for MTR, for the affected Windows versions Looks OK for me. ######################################################## commit b88d54132369b938ba041c0d3f97a5a212210b37 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Mon Nov 22 12:34:10 2021 +0100 MDEV-26713 : Treat codepage 65001 as utf8mb4, not just utf8 Also, fix the "UTF8" option in MSI, which is responsible for character-set-server setting Looks OK for me. ######################################################## commit b9c38470ed6e04185f7f84a056034b7254336d51 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Mon Nov 22 13:22:05 2021 +0100 MDEV-26713 utf8 support on Windows, mysql_upgrade_service part Handle upgrade - on Windows that is capable of UTF8 codepage, convert entries in my.ini from ANSI to UTF8, during upgrade Reason is that paths must now be utf8. For the client , user name, database etc should be in UTF8, too, as this is now the default charset. Should not we add an UTF8 BOM to the beginning of the file, so text editors do not break it later. The patch looks too large for me. It seems it does more than the comments above tell. Looks like convenience refactoring, but it's hard to follow. It would be easier to understand changes if you split it into two parts: 1. the refactoring that adds functions like upgrade_config_file(), fix_section(), is_mariadb_section(), fix_and_check_datadir(), I guess this refactoring is a non-functional change that should not change the behaviour, right? 2. the actual conversion part on top of it ######################################################## commit 0700393b570c6b9a7b4347d3381a7cf05d1499ba Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Tue Nov 23 12:28:28 2021 +0100 mysqltest, Windows - support background execution in 'exec' command If last character in command is ampersand, do not use my_popen/my_pclose in "exec", instead do CreateProcess() without a window. I suggest you add an MDEV for this change and explain in there why we did not need this feature earlier and why we need it now and how it relates to MDEV-26713. ######################################################## commit 7d50d83b3bcab5a3be761f98722e33ed0b01ab23 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Tue Nov 23 13:05:25 2021 +0100 MDEV-26713 utf8 support on Windows , add mysql_install_db tests Add mysql_install_db test with some i18n, for data dir and root password Looks OK for me. ######################################################## commit b2cc09fe76f526bd4cd76131a52263b13b3828a5 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Wed Nov 24 10:15:11 2021 +0100 MDEV-26713 Add test for mysql_install_db creating service, with i18 Looks OK for me. ######################################################## commit 51ce069376fec45d340563a4f7630f8a155afd64 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 26 18:41:35 2021 +0100 MDEV-26713 Introduce --password-codepage client setting I'm not sure we need this new options. Looks like over-engineering a bit. Is the scenario it covers really deserves documenting, learning, etc? I'd better disallow non-ASCII passwords in the future, and right now just add a warning: You're using a non-ASCII password, which is not portable. If you have problems connecting to the database, please ask the administrator to change the password. ######################################################## commit b155c4036cfb3ffdaa7c272048ac9d32e3182d78 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Mon Nov 29 19:47:36 2021 +0100 MDEV-26713 set console codepage to what user set in --default-character-set I prefer to keep the comment that you removed: - if (!fs_cset_cache) - { - char buf[10]= "cp"; - GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_IDEFAULTANSICODEPAGE, - buf+2, sizeof(buf)-3); - /* - We cannot call get_charset_by_name here - because fs_character_set() is executed before - LOCK_THD_charset mutex initialization, which - is used inside get_charset_by_name. - As we're now interested in cp932 only, - let's just detect it using strcmp(). - */ - fs_cset_cache= - #ifdef HAVE_CHARSET_cp932 - !strcmp(buf, "cp932") ? &my_charset_cp932_japanese_ci : - #endif - &my_charset_bin; This function uses both default_charset and charset_info->cs_name.str: +static void adjust_console_codepage() +{ +#ifdef _WIN32 + const char *name= charset_info->cs_name.str; + if (my_set_console_cp(default_charset) < 0) And it seems the former is set to the latter just before the function call: default_charset= (char *)charset_info->cs_name.str; <------ HERE put_info("Charset changed", INFO_INFO); adjust_console_codepage(); I find this hard to follow. Can you please add a parameter to adjust_console_codepage() and pass either charset_info->cs_name.str (or default_charset) when calling? Also, I think this function should only be called when default_charset was some explicit name, not MYSQL_AUTODETECT_CHARSET_NAME. I think the logic should be: 1. If --default-character-set=auto, then detect it from the console. 2. If --default-character-set=csname, then set the console CP to csname. Now in case of N1 it seems to detect the character set from the console, then sets the console to what has been detected. Is the last step really needed in case of auto-detection? ######################################################## commit b38c341cc392630c798eb754f87338d37ec95cdf Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Sun Dec 5 17:08:18 2021 +0100 Fix race condition in test. Do not change console codepage in a possibly shared console. Start new console for chcp. Can you extend the comment and tell what actually happened before this change? Greetings.

2 2

[Maria-developers] Review for: MDEV-26938 Support descending indexes internally in InnoDB (server part)
by Sergey Petrunia 11 Dec '21

11 Dec '21

Hi Serg, There is some non-trivial input too, after all. Please find below. > commit da2903f03de039693354176fda836e6642d6d0f0 > Author: Sergei Golubchik <serg(a)mariadb.org> > Date: Wed Nov 24 16:50:21 2021 +0100 > > MDEV-26938 Support descending indexes internally in InnoDB (server part) > > * preserve DESC index property in the parser > * store it in the frm (only for HA_KEY_ALG_BTREE) > * read it from the frm > * show it in SHOW CREATE > * skip DESC indexes in opt_range.cc and opt_sum.cc > * ORDER BY test > == Trivial input == > diff --git a/sql/unireg.cc b/sql/unireg.cc > index 2726b9a68c2..d6449eeca4c 100644 > --- a/sql/unireg.cc > +++ b/sql/unireg.cc > @@ -685,6 +685,14 @@ static uint pack_keys(uchar *keybuff, uint key_count, KEY *keyinfo, > DBUG_PRINT("loop", ("flags: %lu key_parts: %d key_part: %p", > key->flags, key->user_defined_key_parts, > key->key_part)); > + /* For SPATIAL, FULLTEXT and HASH indexes (anything other than B-tree), > + ignore the ASC/DESC attribute of columns. */ > + const uchar ha_reverse_sort= key->algorithm == HA_KEY_ALG_BTREE > + ? HA_REVERSE_SORT : > + (key->algorithm == HA_KEY_ALG_UNDEF && > + !(key->flags & (HA_FULLTEXT | HA_SPATIAL))) > + ? HA_REVERSE_SORT : 0; The above is hard to read for me. I think if-statement form would be more readable: const uchar ha_reverse_sort= 0; if (key->algorithm == HA_KEY_ALG_BTREE || (key->algorithm == HA_KEY_ALG_UNDEF && !(key->flags & (HA_FULLTEXT | HA_SPATIAL)) ha_reverse_sort= HA_REVERSE_SORT; == Issue #1: ordered index scan is not handled in ha_partition == Testcase: create table t10 (a int, b int, key(a desc)) partition by hash(a) partitions 4; insert into t10 select seq, seq from seq_0_to_999; MariaDB [test]> select * from t10 order by a limit 3; +------+------+ | a | b | +------+------+ | 3 | 3 | | 7 | 7 | | 11 | 11 | +------+------+ 3 rows in set (0.002 sec) Correct output: MariaDB [test]> select * from t10 use index() order by a limit 3; +------+------+ | a | b | +------+------+ | 0 | 0 | | 1 | 1 | | 2 | 2 | +------+------+ 3 rows in set (0.011 sec) == Issue #2: extra warning == MariaDB [test]> create table t1 (a int, b int, key(a), key(a)); Query OK, 0 rows affected, 1 warning (0.016 sec) MariaDB [test]> show warnings; +-------+------+--------------------------------------------------------------------------------------+ | Level | Code | Message | +-------+------+--------------------------------------------------------------------------------------+ | Note | 1831 | Duplicate index `a_2`. This is deprecated and will be disallowed in a future release | +-------+------+--------------------------------------------------------------------------------------+ 1 row in set (0.000 sec) This is ok. MariaDB [test]> create table t2 (a int, b int, key(a), key(a desc)); Query OK, 0 rows affected, 1 warning (0.004 sec) MariaDB [test]> show warnings; +-------+------+--------------------------------------------------------------------------------------+ | Level | Code | Message | +-------+------+--------------------------------------------------------------------------------------+ | Note | 1831 | Duplicate index `a_2`. This is deprecated and will be disallowed in a future release | +-------+------+--------------------------------------------------------------------------------------+ 1 row in set (0.000 sec) This is probably not? MySQL doesn't produce this warning in this case. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

2 3