March 2022 - developers - lists.mariadb.org

Re: [Maria-developers] 349283c5e7a: MDEV-17124: mariadb 10.1.34, views and prepared statements: ERROR 1615 (HY000): Prepared statement needs to be re-prepared
by Sergei Golubchik 27 Sep '22

27 Sep '22

Hi, Oleksandr, On Jan 27, Oleksandr Byelkin wrote: > commit 349283c5e7a > Author: Oleksandr Byelkin <sanja(a)mariadb.com> > Date: Wed Apr 17 15:50:59 2019 +0200 > > MDEV-17124: mariadb 10.1.34, views and prepared statements: ERROR 1615 (HY000): Prepared statement needs to be re-prepared > > The problem is that if table definition cache (TDC) is full of real tables > which are in tables cache, view definition can not stay there so will be > removed by its own underlying tables. > In situation above old mechanism of detection matching definition in PS > and current version always require reprepare and so prevent executing > the PS. > > One work around is to increase TDC, other - improve version check for > views/triggers (which is done here). Now in suspicious cases we check: > - timestamp (ms) of the view to be sure that version really have changed; > - time (ms) of creation of a trigger related to time (ms) of statement > preparation. not "ms" but "us" or "µs". microseconds, not millisecons. > diff --git a/mysql-test/r/ps_ddl.result b/mysql-test/r/ps_ddl.result > index e69c6e06193..005c78a0319 100644 > --- a/mysql-test/r/ps_ddl.result > +++ b/mysql-test/r/ps_ddl.result > @@ -831,8 +831,8 @@ a b c > 20 40 100 > 30 60 150 > call p_verify_reprepare_count(1); > -SUCCESS > - > +ERROR > +Expected: 1, actual: 0 oops, forgot to update? > # Check that we properly handle ALTER VIEW statements. > execute stmt; > a b c > @@ -882,9 +882,9 @@ a b c > 10 50 60 > 20 100 120 > 30 150 180 > -call p_verify_reprepare_count(1); > -SUCCESS > - > +call p_verify_reprepare_count(0); > +ERROR > +Expected: 0, actual: 1 oops, updated too much? > execute stmt; > a b c > 10 50 60 > diff --git a/sql/sql_class.h b/sql/sql_class.h > index 3f0fba8fc10..b9bb9c978fb 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -1079,6 +1079,7 @@ class Statement: public ilink, public Query_arena > > LEX_STRING name; /* name for named prepared statements */ > LEX *lex; // parse tree descriptor > + ulonglong ms_prepare_time; // time of preparation in microseconds "ms" here too :( Better just say my_hrtime_t prepare_time; > /* > Points to the query associated with this statement. It's const, but > we need to declare it char * because all table handlers are written > diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc > index 64e4cd30561..a4a24cc1a80 100644 > --- a/sql/sql_prepare.cc > +++ b/sql/sql_prepare.cc > @@ -4177,6 +4177,9 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) > Query_arena *old_stmt_arena; > DBUG_ENTER("Prepared_statement::prepare"); > DBUG_ASSERT(m_sql_mode == thd->variables.sql_mode); > + > + // The same format as for triggers to compare > + ms_prepare_time= my_hrtime().val; do it at the end instead? > /* > If this is an SQLCOM_PREPARE, we also increase Com_prepare_sql. > However, it seems handy if com_stmt_prepare is increased always, > @@ -4519,6 +4523,22 @@ Prepared_statement::execute_loop(String *expanded_query, > DBUG_ASSERT(thd->get_stmt_da()->sql_errno() == ER_NEED_REPREPARE); > thd->clear_error(); > > + { > + /* > + Check if we too fast with reprepare: > + we can be so fast that: > + 1) make change of a trigger, > + 2) prepare, > + 3) try to exacute and reprepare > + in 1 microsecond, so we will wait till > + next microsecond before last reprepare > + */ > + while (first_prepared == my_hrtime().val) > + { > + pthread_yield(); > + } > + } I wouldn't bother. A more likely case is when a system time goes back, e.g. if there's an ntpd running and your loop doesn't help there. I suggest not to do anything special in this case. And if an ::execute() will open a trigger or a view with a timestamp >= than ms_prepare_time, it will cause reprepare every time. Which is exactly the behavior before your bugfix - opening of a trigger or a view causes reprepare - and it is not a problem unless one has a very small table definition cache. > + > error= reprepare(); > > if (! error) /* Success */ > diff --git a/sql/parse_file.cc b/sql/parse_file.cc > index 999f53bd681..92e7d82af1c 100644 > --- a/sql/parse_file.cc > +++ b/sql/parse_file.cc > @@ -174,11 +174,11 @@ write_parameter(IO_CACHE *file, uchar* base, File_option *parameter) > { > /* string have to be allocated already */ > LEX_STRING *val_s= (LEX_STRING *)(base + parameter->offset); > - time_t tm= my_time(0); > - > - get_date(val_s->str, GETDATE_DATE_TIME|GETDATE_GMT|GETDATE_FIXEDLENGTH, > - tm); > - val_s->length= PARSE_FILE_TIMESTAMPLENGTH; > + ulonglong tm= my_hrtime().val; add a comment here, "number of microseconds since Epoch, timezone-independent" or something like that > + // Paded to 19 characters for compatibility > + val_s->length= snprintf(val_s->str, MICROSECOND_TIMESTAMP_BUFFER_SIZE, > + "%019lld", tm); > + DBUG_ASSERT(val_s->length == MICROSECOND_TIMESTAMP_BUFFER_SIZE-1); > if (my_b_write(file, (const uchar *)val_s->str, > PARSE_FILE_TIMESTAMPLENGTH)) > DBUG_RETURN(TRUE); > @@ -835,15 +835,15 @@ File_parser::parse(uchar* base, MEM_ROOT *mem_root, > /* string have to be allocated already */ > LEX_STRING *val= (LEX_STRING *)(base + parameter->offset); > /* yyyy-mm-dd HH:MM:SS = 19(PARSE_FILE_TIMESTAMPLENGTH) characters */ a comment is now wrong > - if (ptr[PARSE_FILE_TIMESTAMPLENGTH] != '\n') > + if (ptr[MICROSECOND_TIMESTAMP_BUFFER_SIZE-1] != '\n') > { > my_error(ER_FPARSER_ERROR_IN_PARAMETER, MYF(0), > parameter->name.str, line); > DBUG_RETURN(TRUE); > } > - memcpy(val->str, ptr, PARSE_FILE_TIMESTAMPLENGTH); > - val->str[val->length= PARSE_FILE_TIMESTAMPLENGTH]= '\0'; > - ptr+= (PARSE_FILE_TIMESTAMPLENGTH+1); > + memcpy(val->str, ptr, MICROSECOND_TIMESTAMP_BUFFER_SIZE-1); > + val->str[val->length= MICROSECOND_TIMESTAMP_BUFFER_SIZE-1]= '\0'; > + ptr+= MICROSECOND_TIMESTAMP_BUFFER_SIZE; > break; > } > case FILE_OPTIONS_STRLIST: > diff --git a/sql/table.h b/sql/table.h > index 14d6b787b4e..82508388f84 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -2159,7 +2175,7 @@ struct TABLE_LIST > LEX_STRING source; /* source of CREATE VIEW */ > LEX_STRING view_db; /* saved view database */ > LEX_STRING view_name; /* saved view name */ > - LEX_STRING timestamp; /* GMT time stamp of last operation */ > + LEX_STRING ms_timestamp; /* GMT time stamp of last operation */ better keep "timestamp" name. Or "hrtimestamp" if you'd like > st_lex_user definer; /* definer of view */ > ulonglong file_version; /* version of file's field set */ > ulonglong mariadb_version; /* version of server on creation */ > diff --git a/sql/table.cc b/sql/table.cc > index ca6ce02e4f2..24412966b36 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -8480,6 +8480,53 @@ bool TABLE_LIST::is_with_table() > } > > > +bool TABLE_LIST::is_table_ref_id_equal(THD* thd, TABLE_SHARE *s) > +{ > + enum enum_table_ref_type tp= s->get_table_ref_type(); > + if (m_table_ref_type == tp) > + { > + bool res= m_table_ref_version == s->get_table_ref_version(); > + > + /* > + If definition is different check content version > + */ > + if (tabledef_version.length && > + tabledef_version.length == s->tabledef_version.length && > + memcmp(tabledef_version.str, s->tabledef_version.str, > + tabledef_version.length) == 0) > + { > + if (table && table->triggers) > + { > + > + ulonglong ms_stmt_prepare= thd->ms_prepare_time; > + if (ms_stmt_prepare) > + for(uint i= 0; i < TRG_EVENT_MAX; i++) > + for (uint j= 0; j < TRG_ACTION_MAX; j++) > + { > + Trigger *tr= > + table->triggers->get_trigger((trg_event_type)i, > + (trg_action_time_type)j); > + if (tr) > + if (ms_stmt_prepare <= tr->ms_create_time) > + { > + set_tabledef_version(s); > + return FALSE; > + } > + } > + } > + set_table_id(s); > + return TRUE; > + } > + else > + tabledef_version.length= 0; > + return res; > + } > + else > + set_tabledef_version(s); > + return FALSE; > +} this is now a very confusing function. it doesn't only check is_table_ref_id_equal, it also sets some of them (tabledef or both tabledef and ref_id), and under unclear conditions. Could you please comment it? And you don't check that view's timestamp is in the past. If it's not, it can be changed still within the same microsecond. > + > + > uint TABLE_SHARE::actual_n_key_parts(THD *thd) > { > return use_ext_keys && > diff --git a/sql/sql_trigger.h b/sql/sql_trigger.h > index e2e6e1b7acc..70f246d4e0d 100644 > --- a/sql/sql_trigger.h > +++ b/sql/sql_trigger.h > @@ -112,7 +112,7 @@ class Trigger :public Sql_alloc > GRANT_INFO subject_table_grants; > sql_mode_t sql_mode; > /* Store create time. Can't be mysql_time_t as this holds also sub seconds */ > - ulonglong create_time; > + ulonglong ms_create_time; // Create time timestamp in microseconds my_hrtime_t create_time; > trg_event_type event; > trg_action_time_type action_time; > uint action_order; > @@ -195,7 +195,7 @@ class Table_triggers_list: public Sql_alloc > */ > List<ulonglong> definition_modes_list; > /** Create times for triggers */ > - List<ulonglong> create_times; > + List<ulonglong> ms_create_times; ditto > > List<LEX_STRING> definers_list; > > @@ -331,4 +331,14 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create); > extern const char * const TRG_EXT; > extern const char * const TRN_EXT; > > + > +/** > + Make time compatible with MySQL 5.7 trigger time. > +*/ > + > +inline ulonglong make_ms_time(my_time_t time, ulong time_sec_part) make_hrtime. and better put it together with hrtime_to_time, hrtime_from_time, etc > +{ > + return ((ulonglong) time)*1000000 + time_sec_part; > +} > + > #endif /* SQL_TRIGGER_INCLUDED */ > diff --git a/sql/sql_view.cc b/sql/sql_view.cc > index 020830483c5..3b51f6d2451 100644 > --- a/sql/sql_view.cc > +++ b/sql/sql_view.cc > @@ -1131,7 +1141,31 @@ static int mysql_register_view(THD *thd, TABLE_LIST *view, > DBUG_RETURN(error); > } > > +/** > + Check is TABLE_LEST and SHARE match > + @param[in] view TABLE_LIST of the view > + @param[in] share Share object of view > + > + @return false on error or misspatch > +*/ > > +bool mariadb_view_version_get(TABLE_SHARE *share) > +{ > + DBUG_ASSERT(share->is_view); > + > + if (!(share->tabledef_version.str= > + (uchar*) alloc_root(&share->mem_root, > + MICROSECOND_TIMESTAMP_BUFFER_SIZE))) > + return TRUE; > + > + DBUG_ASSERT(share->view_def != NULL); > + if (share->view_def->parse((uchar *) &share->tabledef_version, NULL, > + view_timestamp_parameters, 1, > + &file_parser_dummy_hook)) > + return TRUE; > + DBUG_ASSERT(share->tabledef_version.length == MICROSECOND_TIMESTAMP_BUFFER_SIZE-1); If you open an old view with YYYY-MM-DD... timestamp, where do you convert it? If you open a view without a timestamp at all, where do you handle it? > + return FALSE; > +} > > /** > read VIEW .frm and create structures > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index 9483db9eff9..138fa4bc631 100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -6724,13 +6724,15 @@ static bool store_trigger(THD *thd, Trigger *trigger, > table->field[14]->store(STRING_WITH_LEN("OLD"), cs); > table->field[15]->store(STRING_WITH_LEN("NEW"), cs); > > - if (trigger->create_time) > + if (trigger->ms_create_time) > { > + /* timestamp is in microseconds */ > table->field[16]->set_notnull(); > thd->variables.time_zone->gmt_sec_to_TIME(&timestamp, > - (my_time_t)(trigger->create_time/100)); > - /* timestamp is with 6 digits */ > - timestamp.second_part= (trigger->create_time % 100) * 10000; > + (my_time_t) > + (trigger->ms_create_time/ > + 1000000)); > + timestamp.second_part= trigger->ms_create_time % 1000000; hrtime_to_time() and hrtime_sec_part() > ((Field_temporal_with_date*) table->field[16])->store_time_dec(&timestamp, > 2); > } > @@ -9810,12 +9812,14 @@ static bool show_create_trigger_impl(THD *thd, Trigger *trigger) > trigger->db_cl_name.length, > system_charset_info); > > - if (trigger->create_time) > + if (trigger->ms_create_time) > { > MYSQL_TIME timestamp; > thd->variables.time_zone->gmt_sec_to_TIME(&timestamp, > - (my_time_t)(trigger->create_time/100)); > - timestamp.second_part= (trigger->create_time % 100) * 10000; > + (my_time_t) > + (trigger->ms_create_time/ > + 1000000)); > + timestamp.second_part= trigger->ms_create_time % 1000000; ditto > p->store(&timestamp, 2); > } > else Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 5

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] 49ecf935415: MDEV-27009 Add UCA-14.0.0 collations
by Sergei Golubchik 16 Jun '22

16 Jun '22

Hi, Alexander, On Mar 14, Alexander Barkov wrote: > revision-id: 49ecf935415 (mariadb-10.6.1-335-g49ecf935415) > parent(s): c67789f63c8 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-02-28 14:04:58 +0400 > message: > > MDEV-27009 Add UCA-14.0.0 collations please, list all user visible changes there. Mainly that collations are now decoupled from charsets. New syntax in CREATE TABLE, changes in I_S tables, etc. > diff --git a/mysql-test/include/ctype_utf_uca1400_ids.inc b/mysql-test/include/ctype_utf_uca1400_ids.inc > new file mode 100644 > index 00000000000..09cf49fc0e7 > --- /dev/null > +++ b/mysql-test/include/ctype_utf_uca1400_ids.inc > @@ -0,0 +1,17 @@ file names are confusing. better rename ctype_ucs_uca1400_ids.inc to something like ctype_convert_uca1400_ids and ctype_utf_uca1400_ids to ctype_set_names_uca1400_ids or something like that, to show what they do. > + > +--disable_ps_protocol > +--enable_metadata > +DELIMITER $$; > +FOR rec IN (SELECT COLLATION_NAME > + FROM INFORMATION_SCHEMA.COLLATION_CHARACTER_SET_APPLICABILITY > + WHERE CHARACTER_SET_NAME=@charset > + AND COLLATION_NAME RLIKE 'uca1400' > + ORDER BY ID) > +DO > + EXECUTE IMMEDIATE CONCAT('SET NAMES ',@charset,' COLLATE ', rec.COLLATION_NAME); > + SELECT rec.COLLATION_NAME; > +END FOR; > +$$ > +DELIMITER ;$$ > +--disable_metadata > +--enable_ps_protocol > diff --git a/include/m_ctype.h b/include/m_ctype.h > index 4c6628b72b3..706764ead2a 100644 > --- a/include/m_ctype.h > +++ b/include/m_ctype.h > @@ -34,7 +34,9 @@ enum loglevel { > extern "C" { > #endif > > -#define MY_CS_NAME_SIZE 32 > +#define MY_CS_CHARACTER_SET_NAME_SIZE 32 > +#define MY_CS_COLLATION_NAME_SIZE 64 That's FULL_COLLATION_NAME_SIZE, right? > + > #define MY_CS_CTYPE_TABLE_SIZE 257 > #define MY_CS_TO_LOWER_TABLE_SIZE 256 > #define MY_CS_TO_UPPER_TABLE_SIZE 256 > @@ -240,6 +242,46 @@ typedef enum enum_repertoire_t > } my_repertoire_t; > > > +/* ID compatibility */ > +typedef enum enum_collation_id_type > +{ > + MY_COLLATION_ID_TYPE_PRECISE= 0, > + MY_COLLATION_ID_TYPE_COMPAT_100800= 1 > +} my_collation_id_type_t; > + > + > +/* Collation name display modes */ > +typedef enum enum_collation_name_mode > +{ > + MY_COLLATION_NAME_MODE_FULL= 0, > + MY_COLLATION_NAME_MODE_CONTEXT= 1 > +} my_collation_name_mode_t; > + > + > +/* Level flags */ > +#define MY_CS_LEVEL_BIT_PRIMARY 0x00 > +#define MY_CS_LEVEL_BIT_SECONDARY 0x01 > +#define MY_CS_LEVEL_BIT_TERTIARY 0x02 > +#define MY_CS_LEVEL_BIT_QUATERNARY 0x03 > + > +#define MY_CS_COLL_LEVELS_S1 (1<<MY_CS_LEVEL_BIT_PRIMARY) > + > +#define MY_CS_COLL_LEVELS_AI_CS (1<<MY_CS_LEVEL_BIT_PRIMARY)| \ > + (1<<MY_CS_LEVEL_BIT_TERTIARY) > + > +#define MY_CS_COLL_LEVELS_S2 (1<<MY_CS_LEVEL_BIT_PRIMARY)| \ > + (1<<MY_CS_LEVEL_BIT_SECONDARY) > + > +#define MY_CS_COLL_LEVELS_S3 (1<<MY_CS_LEVEL_BIT_PRIMARY)| \ > + (1<<MY_CS_LEVEL_BIT_SECONDARY) | \ > + (1<<MY_CS_LEVEL_BIT_TERTIARY) AI_CS and S3 don't seem to be used yet > + > +#define MY_CS_COLL_LEVELS_S4 (1<<MY_CS_LEVEL_BIT_PRIMARY)| \ > + (1<<MY_CS_LEVEL_BIT_SECONDARY) | \ > + (1<<MY_CS_LEVEL_BIT_TERTIARY) | \ > + (1<<MY_CS_LEVEL_BIT_QUATERNARY) > + > + > /* Flags for strxfrm */ > #define MY_STRXFRM_LEVEL1 0x00000001 /* for primary weights */ > #define MY_STRXFRM_LEVEL2 0x00000002 /* for secondary weights */ > diff --git a/sql/sql_alter.cc b/sql/sql_alter.cc > index 86c6e9a27f8..9ddd482ad57 100644 > --- a/sql/sql_alter.cc > +++ b/sql/sql_alter.cc > @@ -546,6 +546,7 @@ bool Sql_cmd_alter_table::execute(THD *thd) > > result= mysql_alter_table(thd, &select_lex->db, &lex->name, > &create_info, > + lex->create_info.default_charset_collation, I don't see why you need a new argument here. It's create_info.default_charset_collation, so, mysql_alter_table already gets it in create_info. All other mysql_alter_table invocations also take create_info argument and can get default_charset_collation from there > first_table, > &alter_info, > select_lex->order_list.elements, > diff --git a/sql/sql_partition_admin.cc b/sql/sql_partition_admin.cc > index fb1ae0d5fc7..4188dde252b 100644 > --- a/sql/sql_partition_admin.cc > +++ b/sql/sql_partition_admin.cc > @@ -211,6 +211,7 @@ bool compare_table_with_partition(THD *thd, TABLE *table, TABLE *part_table, > part_table->use_all_columns(); > table->use_all_columns(); > if (unlikely(mysql_prepare_alter_table(thd, part_table, &part_create_info, > + Lex_maybe_default_charset_collation(), Same. Can be in part_create_info > &part_alter_info, &part_alter_ctx))) > { > my_error(ER_TABLES_DIFFERENT_METADATA, MYF(0)); > diff --git a/sql/sql_i_s.h b/sql/sql_i_s.h > index bed2e886718..5ff06d32231 100644 > --- a/sql/sql_i_s.h > +++ b/sql/sql_i_s.h > @@ -162,6 +162,11 @@ class Yesno: public Varchar > { > public: > Yesno(): Varchar(3) { } > + static LEX_CSTRING value(bool val) > + { > + return val ? Lex_cstring(STRING_WITH_LEN("Yes")) : > + Lex_cstring(); > + } eh... please, rename the class from Yesno to something like Yesempty or Yes_or_empty, something that says that the second should not be Lex_cstring(STRING_WITH_LEN("No")) > }; > > > diff --git a/sql/table.cc b/sql/table.cc > index a683a78ff49..c28cb2bd928 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -3491,6 +3493,16 @@ int TABLE_SHARE::init_from_sql_statement_string(THD *thd, bool write, > else > thd->set_n_backup_active_arena(arena, &backup); > > + /* > + THD::reset_db() does not set THD::db_charset, > + so it keeps pointing to the character set and collation > + of the current database, rather than the database of the > + new initialized table. Hmm, is that correct? Could you check other invocation of thd->reset_db()? Perhaps they all need to switch charset? In that case it should be done inside THD::reset_db(). Or may be they have to use mysql_change_db_impl() instead? > + Let's call get_default_db_collation() before reset_db(). > + This forces the db.opt file to be loaded. > + */ > + db_cs= get_default_db_collation(thd, db.str); > + > thd->reset_db(&db); > lex_start(thd); > > @@ -3498,6 +3510,11 @@ int TABLE_SHARE::init_from_sql_statement_string(THD *thd, bool write, > sql_unusable_for_discovery(thd, hton, sql_copy)))) > goto ret; > > + if (!(thd->lex->create_info.default_table_charset= > + thd->lex->create_info.default_charset_collation. > + resolved_to_character_set(db_cs, db_cs))) > + DBUG_RETURN(true); How could this (and similar if()'s in other files) fail? > + > thd->lex->create_info.db_type= hton; > #ifdef WITH_PARTITION_STORAGE_ENGINE > thd->work_part_info= 0; // For partitioning > diff --git a/sql/mysys_charset.h b/sql/mysys_charset.h > new file mode 100644 > index 00000000000..86eaeedd432 > --- /dev/null > +++ b/sql/mysys_charset.h > @@ -0,0 +1,44 @@ > +#ifndef MYSYS_CHARSET > +#define MYSYS_CHARSET > + > +/* Copyright (c) 2021, MariaDB Corporation. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; version 2 of the License. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program; if not, write to the Free Software > + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335 USA */ > + > + > +#include "my_sys.h" > + > + > +class Charset_loader_mysys: public MY_CHARSET_LOADER > +{ > +public: > + Charset_loader_mysys() > + { > + my_charset_loader_init_mysys(this); > + } > + void raise_unknown_collation_error(const char *name, > + CHARSET_INFO *name_cs) const; > + CHARSET_INFO *get_charset(const char *cs_name, uint cs_flags, myf my_flags); > + CHARSET_INFO *get_exact_collation(const char *name, myf utf8_flag); > + CHARSET_INFO *get_contextually_typed_collation(CHARSET_INFO *cs, > + const char *name); > + CHARSET_INFO *get_contextually_typed_collation(const char *name); > + CHARSET_INFO *get_contextually_typed_collation_or_error(CHARSET_INFO *cs, > + const char *name); > + CHARSET_INFO *find_default_collation(CHARSET_INFO *cs); > + CHARSET_INFO *find_bin_collation_or_error(CHARSET_INFO *cs); > +}; you can have C++ code in mysys too, you know, no need to put it in sql/mysys* > + > +#endif // MYSYS_CHARSET > + > diff --git a/strings/ctype-simple.c b/strings/ctype-simple.c > index b579f0af203..d09dfba86ed 100644 > --- a/strings/ctype-simple.c > +++ b/strings/ctype-simple.c > @@ -1940,13 +1941,26 @@ my_bool my_propagate_complex(CHARSET_INFO *cs __attribute__((unused)), > } > > > +void my_ci_set_strength(struct charset_info_st *cs, uint strength) > +{ > + DBUG_ASSERT(strength > 0 && strength <= MY_STRXFRM_NLEVELS); don't use && in asserts, please create two separate asserts instead: DBUG_ASSERT(strength > 0); DBUG_ASSERT(strength <= MY_STRXFRM_NLEVELS); > + cs->levels_for_order= ((1 << strength) - 1); why do you still use the old concept of "strength"? Why not to use bitmap consistently everywhere? > +} > + > + > +void my_ci_set_level_flags(struct charset_info_st *cs, uint flags) > +{ > + DBUG_ASSERT(flags < (1<<MY_STRXFRM_NLEVELS)); > + cs->levels_for_order= flags; > +} > + > /* > Normalize strxfrm flags > > SYNOPSIS: > my_strxfrm_flag_normalize() > + cs - the CHARSET_INFO pointer > flags - non-normalized flags > - nlevels - number of levels > > NOTES: > If levels are omitted, then 1-maximum is assumed. > diff --git a/sql/handler.h b/sql/handler.h > index 8ad521e189a..1e82f37b1e7 100644 > --- a/sql/handler.h > +++ b/sql/handler.h > @@ -2409,7 +2386,32 @@ struct Table_specification_st: public HA_CREATE_INFO, > { > HA_CREATE_INFO::options= 0; > DDL_options_st::init(); > + default_charset_collation.init(); > + } > + > + bool > + add_alter_list_item_convert_to_charset(const Lex_charset_collation_st &cl) > + { > + /* > + cs cannot be NULL, as sql_yacc.yy translates > + CONVERT TO CHARACTER SET DEFAULT > + to > + CONVERT TO CHARACTER SET <character-set-of-the-current-database> > + TODO: Shouldn't we postpone resolution of DEFAULT until the > + character set of the table owner database is loaded from its db.opt? > + */ > + DBUG_ASSERT(cl.charset_collation()); > + DBUG_ASSERT(!cl.is_contextually_typed_collation()); > + alter_table_convert_to_charset= cl.charset_collation(); > + default_charset_collation.Lex_charset_collation_st::operator=(cl); looks quite ugly. can you do, like, default_charset_collation.set(cl) ? > + used_fields|= (HA_CREATE_USED_CHARSET | HA_CREATE_USED_DEFAULT_CHARSET); > + return false; > } > + bool add_table_option_default_charset(CHARSET_INFO *cs); > + bool add_table_option_default_collation(const Lex_charset_collation_st &cl); > + bool resolve_db_charset_and_collation(THD *thd, > + const LEX_CSTRING &db, > + bool is_alter); > }; > > > diff --git a/strings/ctype-uca1400data.h b/strings/ctype-uca1400data.h > new file mode 100644 > index 00000000000..da95dcfde54 > --- /dev/null > +++ b/strings/ctype-uca1400data.h > @@ -0,0 +1,44151 @@ > +/* > + Generated from allkeys.txt version '14.0.0' > +*/ if it's generated, do you need to check it in? perhaps it should be generated during the build? you've checked in allkeys1400.txt anyway. > +static const uint16 uca1400_p000[]= { /* 0000 (4 weights per char) */ > +0x0000,0x0000,0x0000,0x0000, 0x0000,0x0000,0x0000,0x0000, /* 0000 */ > +0x0000,0x0000,0x0000,0x0000, 0x0000,0x0000,0x0000,0x0000, /* 0002 */ > diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc > index 6ca10267187..d115401a855 100644 > --- a/sql/sql_lex.cc > +++ b/sql/sql_lex.cc > @@ -542,6 +542,30 @@ bool LEX::add_alter_list(LEX_CSTRING name, LEX_CSTRING new_name, bool exists) > } > > > +bool LEX::add_alter_list_item_convert_to_charset( > + THD *thd, > + CHARSET_INFO *cs, > + const Lex_charset_collation_st &cl) > +{ > + if (!cs) > + { > + Lex_charset_collation_st tmp; > + tmp.set_charset_collate_default(thd->variables.collation_database); Hmm, what if one is doing ALTER TABLE db.test CHARSET DEFAULT and current db is not `db` but `test` ? > + if (!(cs= tmp.charset_collation())) > + return true; // Should not actually happen assert? > + } > + > + Lex_explicit_charset_opt_collate tmp(cs, false); > + if (tmp.merge_opt_collate_or_error(cl) || > + create_info.add_alter_list_item_convert_to_charset( > + Lex_charset_collation(tmp))) > + return true; > + > + alter_info.flags|= ALTER_CONVERT_TO; > + return false; > +} > + > + > void LEX::init_last_field(Column_definition *field, > const LEX_CSTRING *field_name) > { > @@ -11871,29 +11869,41 @@ CHARSET_INFO *Lex_collation_st::find_default_collation(CHARSET_INFO *cs) > "def" is the upper level CHARACTER SET clause (e.g. of a table) > */ > CHARSET_INFO * > -Lex_collation_st::resolved_to_character_set(CHARSET_INFO *def) const > +Lex_charset_collation_st::resolved_to_character_set(CHARSET_INFO *def) const > { > DBUG_ASSERT(def); > - if (m_type != TYPE_CONTEXTUALLY_TYPED) > - { > - if (!m_collation) > - return def; // Empty - not typed at all > - return m_collation; // Explicitly typed > + > + switch (m_type) { > + case TYPE_EMPTY: > + return def; > + case TYPE_CHARACTER_SET: > + DBUG_ASSERT(m_ci); > + return m_ci; > + case TYPE_COLLATE_EXACT: > + DBUG_ASSERT(m_ci); > + return m_ci; > + case TYPE_COLLATE_CONTEXTUALLY_TYPED: > + break; > } > > // Contextually typed > - DBUG_ASSERT(m_collation); > + DBUG_ASSERT(m_ci); > > - if (m_collation == &my_charset_bin) // CHAR(10) BINARY > - return find_bin_collation(def); > + Charset_loader_mysys loader; > + if (is_contextually_typed_binary_style()) // CHAR(10) BINARY > + return loader.find_bin_collation_or_error(def); > > - if (m_collation == &my_charset_latin1) // CHAR(10) COLLATE DEFAULT > - return find_default_collation(def); > + if (is_contextually_typed_collate_default()) // CHAR(10) COLLATE DEFAULT > + return loader.find_default_collation(def); > + > + const LEX_CSTRING context_name= collation_name_context_suffix(); I'd rather put this in assert, not in if(). Like - if (!strncasecmp(context_name.str, STRING_WITH_LEN("uca1400_"))) + DBUG_ASSERT(!strncasecmp(context_cl_name.str, STRING_WITH_LEN("uca1400_"))); > + if (!strncasecmp(context_name.str, STRING_WITH_LEN("uca1400_"))) > + return loader.get_contextually_typed_collation_or_error(def, > + context_name.str); > > /* > - Non-binary and non-default contextually typed collation. > + Non-binary, non-default, non-uca1400 contextually typed collation. > We don't have such yet - the parser cannot produce this. > - But will have soon, e.g. "uca1400_as_ci". > */ > DBUG_ASSERT(0); > return NULL; > @@ -11944,58 +11972,106 @@ bool Lex_collation_st:: > CHAR(10) BINARY .. COLLATE latin1_bin > CHAR(10) COLLATE uca1400_as_ci .. COLLATE latin1_bin > */ > - if (collation() == &my_charset_latin1 && > - !(cl.collation()->state & MY_CS_PRIMARY)) > + if (is_contextually_typed_collate_default() && > + !cl.charset_collation()->default_flag()) > { > - my_error(ER_CONFLICTING_DECLARATIONS, MYF(0), > - "COLLATE ", "DEFAULT", "COLLATE ", > - cl.collation()->coll_name.str); > + error_conflicting_collations_or_styles(*this, cl); > return true; > } > - if (collation() == &my_charset_bin && > - !(cl.collation()->state & MY_CS_BINSORT)) > + > + if (is_contextually_typed_binary_style() && > + !cl.charset_collation()->binsort_flag()) > { > - my_error(ER_CONFLICTING_DECLARATIONS, MYF(0), > - "", "BINARY", "COLLATE ", cl.collation()->coll_name.str); > + error_conflicting_collations_or_styles(*this, cl); > return true; > } > *this= cl; > return false; > } > > - if (cl.is_contextually_typed_collation()) > - { > + DBUG_ASSERT(0); > + return false; > +} > + > + > +bool > +Lex_explicit_charset_opt_collate:: > + merge_collate_or_error(const Lex_charset_collation_st &cl) > +{ > + DBUG_ASSERT(cl.type() != Lex_charset_collation_st::TYPE_CHARACTER_SET); > + > + switch (cl.type()) { > + case Lex_charset_collation_st::TYPE_EMPTY: > + return false; > + case Lex_charset_collation_st::TYPE_CHARACTER_SET: > + DBUG_ASSERT(0); > + return false; > + case Lex_charset_collation_st::TYPE_COLLATE_EXACT: > /* > - EXPLICIT + CONTEXT > - CHAR(10) COLLATE latin1_bin .. COLLATE DEFAULT - not supported > - CHAR(10) COLLATE latin1_bin .. COLLATE uca1400_as_ci - not yet > + EXPLICIT + EXPLICIT > + CHAR(10) CHARACTER SET latin1 .. COLLATE latin1_bin > + CHAR(10) CHARACTER SET latin1 COLLATE latin1_bin .. COLLATE latin1_bin > + CHAR(10) COLLATE latin1_bin .. COLLATE latin1_bin > + CHAR(10) COLLATE latin1_bin .. COLLATE latin1_bin > + CHAR(10) CHARACTER SET latin1 BINARY .. COLLATE latin1_bin > */ > - DBUG_ASSERT(0); // Not possible yet > + if (m_with_collate && m_ci != cl.charset_collation()) > + { > + my_error(ER_CONFLICTING_DECLARATIONS, MYF(0), > + "COLLATE ", m_ci->coll_name.str, > + "COLLATE ", cl.charset_collation()->coll_name.str); > + return true; > + } > + if (!my_charset_same(m_ci, cl.charset_collation())) > + { > + my_error(ER_COLLATION_CHARSET_MISMATCH, MYF(0), > + cl.charset_collation()->coll_name.str, m_ci->cs_name.str); > + return true; > + } > + m_ci= cl.charset_collation(); > + m_with_collate= true; > return false; > - } > > - /* > - EXPLICIT + EXPLICIT > - CHAR(10) CHARACTER SET latin1 .. COLLATE latin1_bin > - CHAR(10) CHARACTER SET latin1 COLLATE latin1_bin .. COLLATE latin1_bin > - CHAR(10) COLLATE latin1_bin .. COLLATE latin1_bin > - CHAR(10) COLLATE latin1_bin .. COLLATE latin1_bin > - CHAR(10) CHARACTER SET latin1 BINARY .. COLLATE latin1_bin > - */ > - if (type() == TYPE_EXPLICIT && collation() != cl.collation()) > - { > - my_error(ER_CONFLICTING_DECLARATIONS, MYF(0), > - "COLLATE ", collation()->coll_name.str, > - "COLLATE ", cl.collation()->coll_name.str); > - return true; > - } > - if (!my_charset_same(collation(), cl.collation())) > - { > - my_error(ER_COLLATION_CHARSET_MISMATCH, MYF(0), > - cl.collation()->coll_name.str, collation()->cs_name.str); > - return true; > + case Lex_charset_collation_st::TYPE_COLLATE_CONTEXTUALLY_TYPED: > + if (cl.is_contextually_typed_collate_default()) > + { > + /* > + SET NAMES latin1 COLLATE DEFAULT; > + ALTER TABLE t1 CONVERT TO CHARACTER SET latin1 COLLATE DEFAULT; > + */ > + CHARSET_INFO *tmp= Charset_loader_mysys().find_default_collation(m_ci); > + if (!tmp) > + return true; > + m_ci= tmp; > + m_with_collate= true; > + return false; > + } > + else > + { > + /* > + EXPLICIT + CONTEXT > + CHAR(10) COLLATE latin1_bin .. COLLATE DEFAULT not possible yet > + CHAR(10) COLLATE latin1_bin .. COLLATE uca1400_as_ci > + */ > + > + const LEX_CSTRING context_cl_name= cl.collation_name_context_suffix(); > + if (!strncasecmp(context_cl_name.str, STRING_WITH_LEN("uca1400_"))) Like above, better DBUG_ASSERT(!strncasecmp(context_cl_name.str, STRING_WITH_LEN("uca1400_"))) > + { > + CHARSET_INFO *tmp; > + Charset_loader_mysys loader; > + if (!(tmp= loader.get_contextually_typed_collation_or_error(m_ci, > + context_cl_name.str))) > + return true; > + m_with_collate= true; > + m_ci= tmp; > + return false; > + } > + > + DBUG_ASSERT(0); // Not possible yet > + return false; > + } > } > - *this= cl; > + DBUG_ASSERT(0); > return false; > } > > diff --git a/strings/ctype-uca.c b/strings/ctype-uca.c > index b89916f3b20..3e6b4e4ce43 100644 > --- a/strings/ctype-uca.c > +++ b/strings/ctype-uca.c > @@ -30542,7 +30613,7 @@ static const char vietnamese[]= > Myanmar, according to CLDR Revision 8900. > http://unicode.org/cldr/trac/browser/trunk/common/collation/my.xml > */ > -static const char myanmar[]= "[shift-after-method expand][version 5.2.0]" > +static const char myanmar[]= "[shift-after-method expand]" What's going on with myanmar? You removed a version here and added &my_uca_v520 below in its charset_info_st. What does this change mean? > /* Tones */ > "&\\u108C" > "<\\u1037" > @@ -37627,7 +37825,7 @@ struct charset_info_st my_charset_utf32_myanmar_uca_ci= > NULL, /* to_lower */ > NULL, /* to_upper */ > NULL, /* sort_order */ > - NULL, /* uca */ > + &my_uca_v520, /* uca */ What does this change? > NULL, /* tab_to_uni */ > NULL, /* tab_from_uni */ > &my_unicase_unicode520,/* caseinfo */ Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] 9d6be81c0da: MDEV-27832 disable binary logging for SQL SERVICE.
by Sergei Golubchik 05 May '22

05 May '22

Hi, Alexey, On Mar 02, Alexey Botchkov wrote: > revision-id: 9d6be81c0da (mariadb-10.7.2-8-g9d6be81c0da) > parent(s): 33fd136c61b > author: Alexey Botchkov > committer: Alexey Botchkov > timestamp: 2022-02-15 13:37:59 +0400 > message: > > MDEV-27832 disable binary logging for SQL SERVICE. > > Binary logging is now disabled for the queries run by SQL SERVICE. > The binlogging can be turned on with the 'SET SQL_LOG_BIN=On' query. > > diff --git a/mysql-test/suite/plugins/r/test_sql_service.result b/mysql-test/suite/plugins/r/test_sql_service.result > index 00f0411b665..943bb239e5f 100644 > --- a/mysql-test/suite/plugins/r/test_sql_service.result > +++ b/mysql-test/suite/plugins/r/test_sql_service.result > @@ -2,6 +2,19 @@ install plugin test_sql_service soname 'test_sql_service'; > show status like 'test_sql_service_passed'; > Variable_name Value > Test_sql_service_passed 1 > +set global test_sql_service_execute_sql_global= 'create table test.t1 select 1 as a, @@SQL_LOG_BIN'; > +set global test_sql_service_execute_sql_local= 'insert into test.t1 select 2 as a, @@SQL_LOG_BIN'; > +set global test_sql_service_execute_sql_global= 'SET SQL_LOG_BIN=1'; > +set global test_sql_service_execute_sql_global= 'insert into test.t1 select 3 as a, @@SQL_LOG_BIN'; > +set global test_sql_service_execute_sql_global= 'SET SQL_LOG_BIN=0'; > +set global test_sql_service_execute_sql_global= 'insert into test.t1 select 4 as a, @@SQL_LOG_BIN'; > +select * from t1 order by a; this is not a test. Please, enable binlog and see whether queries are actually logged. Also, add a test for, say, sql_auto_is_null: set global test_sql_service_execute_sql_global='set sql_auto_is_null=1'; set global test_sql_service_execute_sql_global='insert test.t1 select 5, @@sql_auto_is_null'; select * from t1 where a=5; > +a @@SQL_LOG_BIN > +1 0 > +2 0 > +3 1 > +4 0 > +drop table t1; > set global test_sql_service_run_test= 1; > show status like 'test_sql_service_passed'; > Variable_name Value > diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc > index e025147c71e..48681ac0b6f 100644 > --- a/sql/sql_prepare.cc > +++ b/sql/sql_prepare.cc > @@ -5617,12 +5617,30 @@ class Protocol_local : public Protocol_text > THD *new_thd; > Security_context empty_ctx; > > + my_bool do_log_bin; > + > Protocol_local(THD *thd_arg, THD *new_thd_arg, ulong prealloc) : > Protocol_text(thd_arg, prealloc), > cur_data(0), first_data(0), data_tail(&first_data), alloc(0), > - new_thd(new_thd_arg) > + new_thd(new_thd_arg), do_log_bin(FALSE) > {} > > + void set_binlog_vars(my_bool *sav_log_bin, ulonglong *sav_bits) > + { > + *sav_log_bin= thd->variables.sql_log_bin; > + *sav_bits= thd->variables.option_bits; > + > + if ((thd->variables.sql_log_bin= do_log_bin)) > + thd->variables.option_bits|= OPTION_BIN_LOG; > + else > + thd->variables.option_bits&= ~OPTION_BIN_LOG; > + } > + void restore_binlog_vars(my_bool &sav_log_bin, ulonglong &sav_bits) > + { > + do_log_bin= thd->variables.sql_log_bin; > + thd->variables.sql_log_bin= sav_log_bin; > + thd->variables.option_bits= sav_bits; that's what sql_auto_is_null test is for.You destroy all other option_bits here. Better don't save/restore option_bits, but use the same if() as in set_binlog_vars(). Or, even better, reuse fix_sql_log_bin_after_update(). I'd suggest to make it a method in THD > + } > protected: > bool net_store_data(const uchar *from, size_t length); > bool net_store_data_cs(const uchar *from, size_t length, > @@ -6230,12 +6248,17 @@ loc_advanced_command(MYSQL *mysql, enum enum_server_command command, > Ed_connection con(p->thd); > Security_context *ctx_orig= p->thd->security_ctx; > MYSQL_LEX_STRING sql_text; > + my_bool log_bin_orig; > + ulonglong option_bits_orig; > + p->set_binlog_vars(&log_bin_orig, &option_bits_orig); > + > DBUG_ASSERT(current_thd == p->thd); > sql_text.str= (char *) arg; > sql_text.length= arg_length; > p->thd->security_ctx= &p->empty_ctx; > result= con.execute_direct(p, sql_text); > p->thd->security_ctx= ctx_orig; > + p->restore_binlog_vars(log_bin_orig, option_bits_orig); > } > if (skip_check) > result= 0; > @@ -6391,6 +6414,9 @@ extern "C" MYSQL *mysql_real_connect_local(MYSQL *mysql) > new_thd->security_ctx->skip_grants(); > new_thd->query_cache_is_applicable= 0; > new_thd->variables.wsrep_on= 0; > + new_thd->variables.sql_log_bin= 0; > + new_thd->variables.option_bits&= ~OPTION_BIN_LOG; > + > /* > TOSO: decide if we should turn the auditing off > for such threads. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 14cc679c95e: MDEV-27831 Let the SQL SERVICE user set the current user name.
by Sergei Golubchik 22 Apr '22

22 Apr '22

Hi, Alexey, On Mar 04, Alexey Botchkov wrote: > revision-id: 14cc679c95e (mariadb-10.7.2-8-g14cc679c95e) > parent(s): 33fd136c61b > author: Alexey Botchkov > committer: Alexey Botchkov > timestamp: 2022-02-15 11:35:18 +0400 > message: > > MDEV-27831 Let the SQL SERVICE user set the current user name. > > The 'user' argument added to the mysql_real_connect_local. I think this is wrong on many levels. A plugin name is already known inside the plugin, you shouldn't force the plugin to pass its own name as an argument in multiple places, the server should determine it automatically. You should not set current_user to an arbitrary string for audit plugin to see it. current_user is the name of the user account and it's used in many places as such. Try, for example, to create a view or a stored procedure. Who will be a definer? Setting only user() might be ok. Setting @@proxy_user or @@external_user is even better, if your audit plugin can show them. @@external_user would be the best, I think it's purely informational. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] 93493a0e9b5: MDEV-24176 Server crashes after insert in the table with virtual column generated using date_format() and if()
by Sergei Golubchik 17 Apr '22

17 Apr '22

Hi, Aleksey! Could you please explain in the commit comment what the bug was and what you are fixing? I wasn't able to understad that, so I had to debug those crashes myself. So, the reason is, objects allocated in the wrong arena, right? And it was done inconsistently, on different code paths the "re-fix" context was different. I agree that encapsulating all the context in one Vcol_expr_context is a good idea, it'll allow to have the same, correct, context everywhere. But always using expr_arena doesn't necessarily seem to be a correct choice. E.g. Arg_comparator creates Item_cache_temporal on every fix_field. If vcol expr is re-fixed all the time, expr_arena will grow continuously. Same for charset conversion, CASE, IN, etc. I suspect that if the vcol expr is always re-fixed, it has to use stmt arena. Why do you re-fix fields? I can hardly imagine that an Item_field can find itself in a different table. And if it's always in the same table, why to force it to find this table over and over? We've already discussed VCOL_TABLE_ALIAS, no need to go there again. On Jan 06, Aleksey Midenkov wrote: > revision-id: 93493a0e9b5 (mariadb-10.2.40-194-g93493a0e9b5) > parent(s): a565e63c54c > author: Aleksey Midenkov > committer: Aleksey Midenkov > timestamp: 2021-12-29 00:46:31 +0300 > message: > > MDEV-24176 Server crashes after insert in the table with virtual > column generated using date_format() and if() > > When table is reopened from cache vcol_info contains stale > expression. We refresh expression via TABLE::vcol_fix_exprs() but > first we must prepare a proper context (Vcol_expr_context) and do some > preparations which meet some requirements: > > 1. fields must not be fixed, we unfix them with cleanup() via > cleanup_processor. > > 2. Item_ident must have cached_table cleared. Again, this is stale > data which was already freed. cached_table interferes with > find_field_in_tables(). > > We cannot clear cached_table directly in Item_ident::cleanup() > method. Lots of spaghetti logic depends on cached_table existence even > after cleanup() done. > > 3. If Item_ident has table_name non-NULL we trigger expr update. That > is done via Item_ident::check_vcol_func_processor() and > VCOL_TABLE_ALIAS flag. > > (4-7) The below conditions are prepared by Vcol_expr_context and used in > both fix_session_expr_for_read() and TABLE::vcol_fix_exprs(): > > 4. Any fix_expr() must be done on expr_arena as there may be new items > created. It was a bug in fix_session_expr_for_read(). It was just not > reproduced because there were no second refix. Now refix is done for > more cases so it does reproduce. Tests affected: vcol.binlog > > 5. Also name resolution context must be narrowed to the single table > with all crutches in place. Tests affected: vcol.update > > 6. sql_mode must be clean and not fail expr update. > > sql_mode such as MODE_NO_BACKSLASH_ESCAPES, MODE_NO_ZERO_IN_DATE, etc > must not affect vcol expression update. If the table was created > successfully any further evaluation must not fail. Tests affected: > main.func_like > > 7. Warnings are disabled during expr update. It is assumed that these > warnings were printed before during initialization phase or later > during execution phase. Tests affected: vcol.wrong_arena > > Dictionary: refresh, update and refix are the synonyms for > vcol_fix_exprs() or fix_session_expr_for_read() calls. > > 8. cleanup_excluding_fields_processor() removed. It was just a quick > hack to exclude wrongly working Item_field from processing. Now it > works due to correct execution environment (Vcol_expr_context). > Related to MDEV-10355. > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 12

Re: [Maria-developers] a10facb656b: MDEV-27065 Partitioning tables with custom data directories moves data back to default directory
by Sergei Golubchik 29 Mar '22

29 Mar '22

Hi, Nayuta, On Mar 29, Nayuta Yanagisawa wrote: > revision-id: a10facb656b (mariadb-10.2.43-48-ga10facb656b) > parent(s): 33ff18627ea > author: Nayuta Yanagisawa > committer: Nayuta Yanagisawa > timestamp: 2022-03-29 23:43:16 +0900 > message: > > MDEV-27065 Partitioning tables with custom data directories moves data back to default directory > > --- > .../suite/parts/r/partition_exch_qa_14.result | 22 ++++++++++++++++++++++ > sql/ha_partition.cc | 16 ++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/mysql-test/suite/parts/r/partition_exch_qa_14.result b/mysql-test/suite/parts/r/partition_exch_qa_14.result > index 1420982436a..52839644f55 100644 > --- a/mysql-test/suite/parts/r/partition_exch_qa_14.result > +++ b/mysql-test/suite/parts/r/partition_exch_qa_14.result > @@ -9,6 +9,9 @@ PARTITION BY RANGE (a) > (PARTITION p0 VALUES LESS THAN (10) DATA DIRECTORY = 'MYSQLTEST_VARDIR/mysql-test-data-dir' INDEX DIRECTORY = 'MYSQLTEST_VARDIR/mysql-test-idx-dir', > PARTITION p1 VALUES LESS THAN (100) DATA DIRECTORY = 'MYSQLTEST_VARDIR/mysql-test-data-dir' INDEX DIRECTORY = 'MYSQLTEST_VARDIR/mysql-test-idx-dir', > PARTITION p2 VALUES LESS THAN (1000) DATA DIRECTORY = 'MYSQLTEST_VARDIR/mysql-test-data-dir' INDEX DIRECTORY = 'MYSQLTEST_VARDIR/mysql-test-idx-dir'); > +Warnings: > +Warning 1618 <DATA DIRECTORY> option ignored > +Warning 1618 <INDEX DIRECTORY> option ignored This is very confusing. There are four DATA DIRECTORY and four INDEX DIRECTORY in the statement. Which one is ignored? (yes, *I* know, the table level one) > @@ -148,6 +154,9 @@ PARTITION BY RANGE (a) > (PARTITION p0 VALUES LESS THAN (10) , > PARTITION p1 VALUES LESS THAN (100) , > PARTITION p2 VALUES LESS THAN (1000) ); > +Warnings: > +Warning 1618 <DATA DIRECTORY> option ignored > +Warning 1618 <INDEX DIRECTORY> option ignored and here DATA/INDEX DIRECTORY behaves not like other options. For a normal option table-level value would apply to all partitions. That is CREATE TABLE t1 (...) OPT="VAL (PARTITION p0 VALUES LESS THAN (10) , PARTITION p1 VALUES LESS THAN (100) , PARTITION p2 VALUES LESS THAN (1000) ); would be the same as CREATE TABLE t1 (...) (PARTITION p0 VALUES LESS THAN (10) OPT="VAL, PARTITION p1 VALUES LESS THAN (100) OPT="VAL, PARTITION p2 VALUES LESS THAN (1000) OPT="VAL); Is that correct? > CREATE TABLE tsp (a INT, b VARCHAR(55), PRIMARY KEY (a)) DATA DIRECTORY = 'MYSQLTEST_VARDIR/mysql-test-data-dir' INDEX DIRECTORY = 'MYSQLTEST_VARDIR/mysql-test-idx-dir' ENGINE = MYISAM > PARTITION BY RANGE (a) > SUBPARTITION BY HASH(a) Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 88f8aa20bba: MDEV-23486 RBR can bypass secure_timestamp=YES
by Sergei Golubchik 26 Mar '22

26 Mar '22

Hi, Aleksey! On Jan 09, Aleksey Midenkov wrote: > revision-id: 88f8aa20bba (mariadb-10.3.31-79-g88f8aa20bba) > parent(s): d5451867af1 > author: Aleksey Midenkov > committer: Aleksey Midenkov > timestamp: 2021-12-23 04:15:39 +0300 > message: > > MDEV-23486 RBR can bypass secure_timestamp=YES > > Pass to a slave whether master table is system-versioned via > TM_SYSTEM_VERSIONED flag. That deprecates m_vers_from_plain > detection. If secure_timestamp == YES or master did not supply row_end > data treat the row as current data and generate timestamps for it. I don't understand why does it matter whether the master is system versioned or not. secure_timestamp=YES means the table has to store actual timestamps when rows were changed. It doesn't trust the master to set timestamps. So it should not matter whether master table is versioned or not, if you don't trust the master, you cannot trust its TM_SYSTEM_VERSIONED flag either. Even if the table was system versioned on the master, the master could've had secure_timestamp=NO and a completely fake history. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] MDEV-24560 SIGSEGV in st_join_table::cleanup
by Sergey Petrunia 25 Mar '22

25 Mar '22

Hi Oleg, > commit 4f41909c3ee1d7e42c56947c9a9156fb78c22d8a > Author: Oleg Smirnov <olernov(a)gmail.com> > Date: Fri Mar 11 21:18:34 2022 +0700 > > MDEV-24560 SIGSEGV in st_join_table::cleanup > > If JOIN::create_postjoin_aggr_table encounters errors during execution > then free_tmp_table() is then called twice for JOIN_TAB::aggr. > The solution is to initialize JOIN_TAB::aggr and JOIN_TAB::table only > on successful completion of JOIN::create_postjoin_aggr_table > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index c044656e916..8c07014d981 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -4037,10 +4037,6 @@ JOIN::create_postjoin_aggr_table(JOIN_TAB *tab, List<Item> *table_fields, > !tables_list); > if (tab > join_tab) > (tab - 1)->next_select= sub_select_postjoin_aggr; > - if (!(tab->aggr= new (thd->mem_root) AGGR_OP(tab))) > - goto err; > - tab->table= table; > - table->reginfo.join_tab= tab; > > /* if group or order on first table, sort first */ > if ((group_list && simple_group) || > @@ -4089,7 +4085,10 @@ JOIN::create_postjoin_aggr_table(JOIN_TAB *tab, List<Item> *table_fields, > order= NULL; > } > } > - > + if (!(tab->aggr= new (thd->mem_root) AGGR_OP(tab))) > + goto err; > + tab->table= table; > + table->reginfo.join_tab= tab; > DBUG_RETURN(false); I see a potential issue in the patch: The moved code does this assignment: > tab->table= table; Let's look inside this if-branch: >> if ((group_list && simple_group) || >> (implicit_grouping && select_lex->have_window_funcs())) The code inside if() { ... } part seems to be fine: it does not touch the contents of 'tab', in particular tab->table. Let's look at the coed inside the else {... } part: It has this condition: >> if (!group_list && !table->distinct && order && simple_order && >> tab == join_tab + const_tables) >> { Note that tab==join_tab+const_tables. Then, a few lines below: >> add_sorting_to_table(join_tab + const_tables, order)) This is essentially add_sorting_to_table(tab, order). Inside JOIN::add_sorting_to_table one can find this: TABLE *table= tab->table; if ((tab == join_tab + const_tables) && table->pos_in_table_list && table->pos_in_table_list->is_sjm_scan_table()) that is, we access tab->table and tab->table->pos_in_table_list. Which are not assigned yet (because your patch moves the assignment to happen later). I've tried to construct an example to demonstrate this issue, but was not successful. I've made this change: diff --git a/sql/sql_select.cc b/sql/sql_select.cc index c044656e916..5a9a7d7e73f 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -4080,6 +4080,8 @@ JOIN::create_postjoin_aggr_table(JOIN_TAB *tab, List<Item> *table_fields, tab == join_tab + const_tables) { DBUG_PRINT("info",("Sorting for order")); + fprintf(stderr,"AAABBBCCC:%s\n", thd->query()); + DBUG_ASSERT(0); THD_STAGE_INFO(thd, stage_sorting_for_order); if (ordered_index_usage != ordered_index_order_by && and ran the entire testsuite with it. There were no failures. I'm not sure if this is deadcode or we just don't have enough test coverage. I suggest to assume the latter and change the patch to avoid this issue. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

Re: [Maria-developers] 3be1e491fb5: MDEV-27743 Remove Lex::charset
by Sergei Golubchik 22 Mar '22

22 Mar '22

Hi, Alexander, All comments from the previous review were addressed and there were no other changes. So, ok to push. On Mar 22, Alexander Barkov wrote: > revision-id: 3be1e491fb5 (mariadb-10.6.1-383-g3be1e491fb5) > parent(s): c69defd3b61 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-03-21 21:33:30 +0400 > message: > > MDEV-27743 Remove Lex::charset > > This patch also fixes: > > MDEV-27690 Crash on `CHARACTER SET csname COLLATE DEFAULT` in column definition > MDEV-27853 Wrong data type on column `COLLATE DEFAULT` and table `COLLATE some_non_default_collation` > MDEV-28067 Multiple conflicting column COLLATE clauses are not rejected > MDEV-28118 Wrong collation of `CAST(.. AS CHAR COLLATE DEFAULT)` > MDEV-28119 Wrong column collation on MODIFY + CONVERT > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 026e52bad84: MDEV-27743 Remove Lex::charset
by Sergei Golubchik 21 Mar '22

21 Mar '22

Hi, Alexander, Please, see below On Mar 18, Alexander Barkov wrote: > revision-id: 026e52bad84 (mariadb-10.6.1-369-g026e52bad84) > parent(s): 369498d6b8c > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-03-18 12:26:03 +0400 > message: > > MDEV-27743 Remove Lex::charset > > This patch also fixes: > > MDEV-27690 Crash on `CHARACTER SET csname COLLATE DEFAULT` in column definition > MDEV-27853 Wrong data type on column `COLLATE DEFAULT` and table `COLLATE some_non_default_collation` > MDEV-28067 Multiple conflicting column COLLATE clauses are not rejected > MDEV-28118 Wrong collation of `CAST(.. AS CHAR COLLATE DEFAULT)` > MDEV-28119 Wrong column collation on MODIFY + CONVERT > > diff --git a/mysql-test/main/ctype_collate_column.test b/mysql-test/main/ctype_collate_column.test > new file mode 100644 > index 00000000000..6a7cf2cc84c > --- /dev/null > +++ b/mysql-test/main/ctype_collate_column.test > @@ -0,0 +1,637 @@ ... > + > +DELIMITER $$; > +CREATE PROCEDURE p1(dt TEXT, cs TEXT, cl0 TEXT, cl1 TEXT, cl2 TEXT, tcs TEXT) > +BEGIN > + DECLARE errstate TEXT DEFAULT NULL; > + DECLARE errno INT DEFAULT NULL; > + DECLARE errmsg TEXT DEFAULT NULL; > + DECLARE query TEXT; > + DECLARE result TEXT; > + DECLARE CONTINUE HANDLER FOR SQLEXCEPTION > + BEGIN > + GET DIAGNOSTICS CONDITION 1 errstate = RETURNED_SQLSTATE, > + errno = MYSQL_ERRNO, errmsg = MESSAGE_TEXT; > + END; > + SET query= CONCAT('CREATE TABLE t1 (a ', dt, ' ', cs, ' ', cl0, > + ' NOT NULL ',cl1, > + ' DEFAULT '''' ', cl2, > + ') ', tcs, ' ENGINE=Memory'); > + EXECUTE IMMEDIATE query; > + IF errmsg IS NOT NULL > + THEN > + SET result=CONCAT('ERROR: ', > + COALESCE(errstate,'<NULL>'), ' ', > + COALESCE(errno,'<NULL>'), ' ', > + COALESCE(errmsg,'<NULL>')); > + INSERT INTO results (dt,cs,cl0,cl1,cl2,tcs,query,result) > + VALUES (dt,cs,cl0,cl1,cl2,tcs,query,result); > + ELSE > + FOR row IN (SELECT CONCAT(COLUMN_TYPE, > + ' CHARACTER SET ', CHARACTER_SET_NAME, > + ' COLLATE ', COLLATION_NAME) AS result > + FROM INFORMATION_SCHEMA.COLUMNS > + WHERE TABLE_SCHEMA='test' AND TABLE_NAME='t1') > + DO > + INSERT INTO results (dt,cs,cl0,cl1,cl2,tcs,query,result) > + VALUES (dt,cs,cl0,cl1,cl2,tcs,query,row.result); > + END FOR; > + DROP TABLE t1; > + END IF; > +END; > +$$ > +DELIMITER ;$$ > + > + > +DELIMITER $$; > +CREATE PROCEDURE p3(dt TEXT) > +BEGIN > + FOR row IN ( > + SELECT cs, cl0, cl1.cl1 AS cl1, cl2.cl1 AS cl2, tcs > + FROM cs, cl0, cl1, cl1 AS cl2, tcs > + ORDER BY cs, cl0, cl1, cl2, tcs > + ) > + DO > + CALL p1('char(10)', row.cs, row.cl0, row.cl1, row.cl2, row.tcs); minor detail: here ^^^ you want to use dt, not 'char(10)' > + END FOR; > +END; > +$$ > +DELIMITER ;$$ > + > + > +--disable_column_names > +CALL p3('char(10)'); > +--enable_column_names > + > +SELECT * FROM results_statistics; isn't it redundant? you select all rows from result anyway. If something will change, the result of that select will already cause the the to fail. Why do you need second mismatch in the result/reject. > + > +--vertical_results > +SELECT query, result, '' AS `` FROM results > +ORDER BY dt, cs, cl0, cl1, cl2, tcs; > +--horizontal_results > + > +DROP PROCEDURE p1; > +DROP PROCEDURE p3; > + > +DROP TABLE cs, cl0, cl1, tcs; ... > +# CREATE TABLE t1 > +# ( > +# a CHAR(10) BINARY NOT NULL DEFAULT '' > +# ) CHARACTER SET cs COLLATE cs_bin; > + > +DELETE FROM results > +WHERE cs='' > + AND cl0='BINARY' > + AND cl1='' > + AND cl2='' > + AND tcs LIKE 'CHARACTER SET%' > + AND result NOT LIKE 'ERROR%' > + AND result RLIKE CONCAT('COLLATE ', tcs_character_set_name, '_bin'); > +SELECT ROW_COUNT(); ... > +DROP FUNCTION is_collate_clause_with_explicit_default_collation; > +DROP FUNCTION is_collate_clause_with_explicit_collation; > +DROP FUNCTION is_conflicting_charset_explicit_collate_explicit; > +DROP FUNCTION is_conflicting_collate_explicit2; > +DROP FUNCTION is_conflicting_collate_default_collate_explicit; > +DROP FUNCTION collate_cs_default_collation; I think the main result is `select * from results` (with order by, etc). It's great, that you added that. Everything else is redundant, I suggest to remove other selects and deletes. > + > + > +--echo # > +--echo # End of 10.9 tests > +--echo # > diff --git a/sql/lex_charset.h b/sql/lex_charset.h > new file mode 100644 > index 00000000000..d95f0bc1920 > --- /dev/null > +++ b/sql/lex_charset.h > @@ -0,0 +1,202 @@ ... > +#define LEX_CHARSET_COLLATION_TYPE_BITS 2 > + static_assert(((1<<LEX_CHARSET_COLLATION_TYPE_BITS)-1) >= > + TYPE_COLLATE_CONTEXTUALLY_TYPED, > + "Lex_charset_collation_st::Type bits check"); > + > +protected: > + CHARSET_INFO *m_ci; > + Type m_type; > +public: > + static CHARSET_INFO *find_bin_collation(CHARSET_INFO *cs); > + static CHARSET_INFO *find_default_collation(CHARSET_INFO *cs); > +public: ... > + void set_contextually_typed_binary_style() > + { > + m_ci= &my_collation_contextually_typed_binary; > + m_type= TYPE_COLLATE_CONTEXTUALLY_TYPED; > + } > + bool is_contextually_typed_collate_default() const > + { > + return m_ci == &my_collation_contextually_typed_default && > + m_type == TYPE_COLLATE_CONTEXTUALLY_TYPED; what does it mean when only one of these two expressions is true? Say, the first true, the second false? Or the first false, the second true? > + } > + bool is_contextually_typed_binary_style() const > + { > + return m_ci == &my_collation_contextually_typed_binary && > + m_type == TYPE_COLLATE_CONTEXTUALLY_TYPED; > + } ... > diff --git a/strings/ctype-bin.c b/strings/ctype-bin.c > index bb746ad90b0..a5c9129ebea 100644 > --- a/strings/ctype-bin.c > +++ b/strings/ctype-bin.c > @@ -630,3 +630,69 @@ struct charset_info_st my_charset_bin = > &my_charset_handler, > &my_collation_binary_handler > }; > + > + > +struct charset_info_st my_collation_contextually_typed_binary= > +{ > + 0,0,0, /* number */ I thought you will never need to resove the &my_collation_contextually_typed_binary pointer, that is, I thought the values in the my_collation_contextually_typed_binary struct should not matter as they'll never be used. Was that wrong? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] d8e915a88fc: MDEV-26009 Server crash when calling twice procedure using FOR-loop
by Sergei Golubchik 20 Mar '22

20 Mar '22

Hi, Oleksandr, ok to push also, see below On Mar 20, Oleksandr Byelkin wrote: > revision-id: d8e915a88fc (mariadb-10.3.33-32-gd8e915a88fc) > parent(s): 6a2d88c1322 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2022-03-18 11:13:09 +0100 > message: > > MDEV-26009 Server crash when calling twice procedure using FOR-loop > > The problem was that instructions sp_instr_cursor_copy_struct and > sp_instr_copen uses the same lex, adding and removing "tail" of > prelocked tables and forgetting that tail of all tables is kept in > LEX::query_tables_last. If the LEX used only by one instruction > or the query do not have prelocked tables it is not important. > But to work correctly in all cases LEX::query_tables_last should > be reset to make new tables added in the correct list (after last > table in the LEX instead after last table of the prelocking "tail" > which was cut). > > diff --git a/sql/sp_head.cc b/sql/sp_head.cc > index 57ab31d9edf..96dc78dcf49 100644 > --- a/sql/sp_head.cc > +++ b/sql/sp_head.cc > @@ -3486,6 +3486,11 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp, > lex_query_tables_own_last= m_lex->query_tables_own_last; > prelocking_tables= *lex_query_tables_own_last; > *lex_query_tables_own_last= NULL; > + /* > + Here we cut "tail" of prelocked tables, so have to return last table > + to the original position (wuthout the tail > + */ > + m_lex->query_tables_last= m_lex->query_tables_own_last; 1. the comment looks truncated 2. I don't even think this line needs a comment, this code block removes the list of prelocked tables from the query_tables list. Looking at how query_tables_last and query_tables_own_last are documented: /* Pointer to next_global member of last element in the previous list. */ TABLE_LIST **query_tables_last; /* If non-0 then indicates that query requires prelocking and points to next_global member of last own element in query table list (i.e. last table which was not added to it as part of preparation to prelocking). 0 - indicates that this query does not need prelocking. */ TABLE_LIST **query_tables_own_last; it is pretty clear that after the "tail" is removed, one has to set query_tables_last to point to the end of the list. The fact that it wasn't done was clearly an omission. That is, existing code and existing comments document it pretty well, your comment looks redundant and even confusing to me. So, please, remove the comment and ok to push > m_lex->mark_as_requiring_prelocking(NULL); > } > thd->rollback_item_tree_changes(); Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] Google Summer of Code 2022
by Vicențiu Ciorbaru 20 Mar '22

20 Mar '22

Hi Sergey! I'm happy to see that you have experience in MariaDB code base already. The MDEV-18827 is definitely up for grabs for GSoC 2022. The project is mostly a part-time project (requiring a commitment of around 175 hours), however depending on your availability we can increase the scope of the project and bump it up to a full time project. My suggestion for this task is to start looking at code parsing FRM files. Search for the function TABLE_SHARE::init_from_binary_frm_image to get an idea how an .frm is structured. After that, think how you would like to implement the feature described in MDEV-18827. Feel free to submit a draft proposal and we can iterate on it. Not 100% sure how the GSoC interface works this year, but last time you were able to share with potential mentors a Google Doc with the possibility to comment. Have a look at the proposal guidelines https://mariadb.org/gsoc2022-instructions/ and start writing it. When the program application opens you can submit it (4th of April). In the meantime, feel free to ask any questions you might have here. I encourage you to keep all discussions on the mailing list as much as possible so others can chime in. Vicențiu On Sat, 19 Mar 2022 at 09:54, Сергей Ваниславский <serzhmn(a)gmail.com> wrote: > Hello Vicentiu! > My name is Vanislavsky Sergey. Your colleague Nikita Malyavin > recommended contacting you about GSoC.I would like to do your task > MDEV-18827. I am a 3rd year student at FEFU. Here is my last pull request: > https://github.com/MariaDB/server/pull/2016 > Thank you for your consideration. >

1 0

[Maria-developers] Relevance of MDEV-19556
by Vladisav Kakurin 17 Mar '22

17 Mar '22

Hi, Vicențiu! With my mentor Nikita Malyavin we chose the topic of my bachelor's graduation work, and I was really interested in the MDEV-19556 <https://jira.mariadb.org/browse/MDEV-19556> ticket. Could you say some words about this, maybe the ticket ain’t actual, or something about work that was done by one of the developers to this direction?

1 0

Re: [Maria-developers] 2467eb2d314: MDEV-27743 Remove Lex::charset
by Sergei Golubchik 16 Mar '22

16 Mar '22

Hi, Alexander, I had only a couple of questions/comments, see below: On Mar 11, Alexander Barkov wrote: > revision-id: 2467eb2d314 (mariadb-10.6.1-330-g2467eb2d314) > parent(s): 61b72ed3dde > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-02-10 10:27:55 +0400 > message: > > MDEV-27743 Remove Lex::charset > > This patch also fixes: > MDEV-27690 Crash on `CHARACTER SET csname COLLATE DEFAULT` in column definition how do you plan to fix it in 10.2-10.8 ? > > diff --git a/mysql-test/main/ctype_collate_column.result b/mysql-test/main/ctype_collate_column.result > new file mode 100644 > index 00000000000..b20be5d3a48 > --- /dev/null > +++ b/mysql-test/main/ctype_collate_column.result > @@ -0,0 +1,450 @@ > +# > +# Start of 10.9 tests > +# > +# > +# MDEV-27743 Remove Lex::charset 1. Jira doesn't explain what user-visible changes one can expect. If there are none, it's not clear why you needed this big test case 2. the test is totally unreadable. Could you do a normal test instead? Like, put `select` instead of `execute immediate` and convert the output to a .test file? > +# > +CREATE TABLE cs (cs VARCHAR(64) NOT NULL); > +INSERT INTO cs (cs) VALUES > +(''), > +('CHARACTER SET latin1'), > +('CHARACTER SET utf8mb4'); > +CREATE TABLE cl0 (cl0 VARCHAR(64) NOT NULL); > +INSERT INTO cl0 (cl0) VALUES > +(''), > +('BINARY'), > +('COLLATE DEFAULT'), > +('COLLATE latin1_bin'), > +('COLLATE latin1_swedish_ci'), > +('COLLATE utf8mb4_bin'), > +('COLLATE utf8mb4_general_ci'); > +CREATE TABLE cl1 (cl1 VARCHAR(64) NOT NULL); > +INSERT INTO cl1 (cl1) VALUES > +(''), > +('COLLATE DEFAULT'), > +('COLLATE latin1_bin'), > +('COLLATE latin1_swedish_ci'), > +('COLLATE utf8mb4_bin'), > +('COLLATE utf8mb4_general_ci'); ... > diff --git a/mysql-test/main/ctype_latin1.result b/mysql-test/main/ctype_latin1.result > index 0b16d1cf6f2..70577f2c22c 100644 > --- a/mysql-test/main/ctype_latin1.result > +++ b/mysql-test/main/ctype_latin1.result > @@ -8889,6 +8889,38 @@ a b > 111 111 > DROP TABLE t1; > # > +# MDEV-27690 Crash on `CHARACTER SET csname COLLATE DEFAULT` in column definition > +# > +CREATE TABLE t1 (a CHAR(10) CHARACTER SET latin1 COLLATE DEFAULT); > +SHOW CREATE TABLE t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `a` char(10) DEFAULT NULL > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +DROP TABLE t1; > +SELECT CAST('a' AS CHAR(10) CHARACTER SET latin1 COLLATE DEFAULT); > +CAST('a' AS CHAR(10) CHARACTER SET latin1 COLLATE DEFAULT) > +a > +CREATE TABLE t1 AS > +SELECT CAST('a' AS CHAR(10) CHARACTER SET latin1 COLLATE DEFAULT) AS c1; > +SHOW CREATE TABLE t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `c1` varchar(10) DEFAULT NULL > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +DROP TABLE t1; > +SELECT COLUMN_GET(COLUMN_CREATE(0, 'string'),0 AS CHAR CHARACTER SET latin1 COLLATE DEFAULT) AS c1; > +c1 > +string > +CREATE TABLE t1 AS > +SELECT COLUMN_GET(COLUMN_CREATE(0, 'string'),0 AS CHAR CHARACTER SET latin1 COLLATE DEFAULT) AS c1; > +SHOW CREATE TABLE t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `c1` longtext DEFAULT NULL > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +DROP TABLE t1; 1. why is it in "10.2 tests" ? 2. shouldn't here be at least one test that COLLATE DEFAULT is not a no-op? Like CREATE TABLE (... COLLATE DEFAULT ...) COLLATE _non_default ? Or is it in your generated ctype_collate_column.test? > +# > # End of 10.2 tests > # > # > diff --git a/mysql-test/suite/roles/create_and_drop_role_invalid_user_table.test b/mysql-test/suite/roles/create_and_drop_role_invalid_user_table.test > index 9cf0d7b4aff..dac6eab21e9 100644 > --- a/mysql-test/suite/roles/create_and_drop_role_invalid_user_table.test > +++ b/mysql-test/suite/roles/create_and_drop_role_invalid_user_table.test > @@ -36,7 +36,7 @@ set default role test_role for root@localhost; > drop role test_role; > drop user test_user@localhost; > > -alter table user add column default_role char(80) binary default '' not null > +alter table user add column default_role char(80) default '' not null > COLLATE utf8_general_ci Heh, interesting that it worked before good catch > after is_role; > alter table user add max_statement_time decimal(12,6) default 0 not null > diff --git a/sql/structs.h b/sql/structs.h > index 9a5006e8b47..db065c49931 100644 > --- a/sql/structs.h > +++ b/sql/structs.h > @@ -599,11 +599,140 @@ class DDL_options: public DDL_options_st ... > +class Lex_collation: public Lex_collation_st > +{ > +public: > + Lex_collation(CHARSET_INFO *collation, Type type) > + { > + m_collation= collation; > + m_type= type; > + } > + static Lex_collation national(bool bin_mod) > + { > + if (bin_mod) > + return Lex_collation(&my_charset_utf8mb3_bin, TYPE_EXPLICIT); > + return Lex_collation(&my_charset_utf8mb3_general_ci, TYPE_IMPLICIT); utf8mb3? not utf8? > + } > +}; > + > + > struct Lex_length_and_dec_st > { > -private: > +protected: > uint32 m_length; > uint8 m_dec; > + uint8 m_collation_type:2; > bool m_has_explicit_length:1; > bool m_has_explicit_dec:1; > public: > diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc > index ae6ae9a0cc0..6ca10267187 100644 > --- a/sql/sql_lex.cc > +++ b/sql/sql_lex.cc > @@ -11816,3 +11784,257 @@ bool SELECT_LEX_UNIT::explainable() const ... > +CHARSET_INFO * > +Lex_collation_st::resolved_to_character_set(CHARSET_INFO *def) const > +{ > + DBUG_ASSERT(def); > + if (m_type != TYPE_CONTEXTUALLY_TYPED) > + { > + if (!m_collation) > + return def; // Empty - not typed at all > + return m_collation; // Explicitly typed > + } > + > + // Contextually typed > + DBUG_ASSERT(m_collation); > + > + if (m_collation == &my_charset_bin) // CHAR(10) BINARY > + return find_bin_collation(def); > + > + if (m_collation == &my_charset_latin1) // CHAR(10) COLLATE DEFAULT > + return find_default_collation(def); Uhm. Could you create two dummy CHARSET_INFO variables for that instead? Like CHARSET_INFO binary_collaton, default_collation; no need to initialize them, but to have distinct values for binary/default and not reuse my_charset_bin and my_charset_latin1. > + > + /* > + Non-binary and non-default contextually typed collation. > + We don't have such yet - the parser cannot produce this. > + But will have soon, e.g. "uca1400_as_ci". > + */ > + DBUG_ASSERT(0); > + return NULL; > +} > + > + ... > diff --git a/sql/field.h b/sql/field.h > index 2ed02b37cfd..fe5ba435202 100644 > --- a/sql/field.h > +++ b/sql/field.h > @@ -5493,6 +5492,22 @@ class Column_definition: public Sql_alloc, > { return compression_method_ptr; } > > bool check_vcol_for_key(THD *thd) const; > + > + void set_lex_collation(const Lex_collation_st &lc) > + { > + charset= lc.collation(); > + if (lc.is_contextually_typed_collation()) > + flags|= BINCMP_FLAG; > + else > + flags&= ~BINCMP_FLAG; Isn't COLLATE DEFAULT also contextually typed? > + } > + Lex_collation lex_collation() const > + { > + return Lex_collation(charset, > + flags & BINCMP_FLAG ? > + Lex_collation_st::TYPE_CONTEXTUALLY_TYPED : > + Lex_collation_st::TYPE_IMPLICIT); > + } > }; > > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] 75aa95ada6b: Tricky static asserts did not compile on some platforms. Commenting out.
by Sergei Golubchik 16 Mar '22

16 Mar '22

Hi, Alexander, On Mar 16, Alexander Barkov wrote: > revision-id: 75aa95ada6b (mariadb-10.6.1-338-g75aa95ada6b) > parent(s): e90062c20f1 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-03-09 13:07:35 +0400 > message: > > Tricky static asserts did not compile on some platforms. Commenting out. Details? What doesn't compile and where? I'd suggest to try to keep the assert in some form > diff --git a/sql/structs.h b/sql/structs.h > index abfafb635b1..23958490c7b 100644 > --- a/sql/structs.h > +++ b/sql/structs.h > @@ -602,13 +602,16 @@ class DDL_options: public DDL_options_st > > struct Lex_length_and_dec_st > { > + /* > + Does not compile on some platforms: > static constexpr uint collation_type_bits= 2; > static_assert(((1<<collation_type_bits)-1) >= > Lex_charset_collation_st::TYPE_LAST); > + */ > protected: > uint32 m_length; > uint8 m_dec; > - uint8 m_collation_type:collation_type_bits; > + uint8 m_collation_type:2; /*collation_type_bits;*/ > bool m_has_explicit_length:1; > bool m_has_explicit_dec:1; > public: Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] e90062c20f1: MDEV-27266 Improve UCA collation performance for utf8mb3 and utf8mb4
by Sergei Golubchik 16 Mar '22

16 Mar '22

Hi, Alexander, also ok On Mar 14, Alexander Barkov wrote: > revision-id: e90062c20f1 (mariadb-10.6.1-337-ge90062c20f1) > parent(s): 6a3201d3769 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-02-28 14:43:43 +0400 > message: > > MDEV-27266 Improve UCA collation performance for utf8mb3 and utf8mb4 > > Adding two levels of optimization: > > include/m_ctype.h | 53 ++++ > strings/ctype-uca-scanner_next.inl | 39 +++ > strings/ctype-uca.c | 548 ++++++++++++++++++++++++++++++++++++- > strings/ctype-uca.ic | 30 ++ I thought all *.ic files were renamed to *.inl ? > unittest/strings/strings-t.c | 2 +- > 5 files changed, 660 insertions(+), 12 deletions(-) Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 6a3201d3769: MDEV-27265 Improve contraction performance in UCA collations
by Sergei Golubchik 16 Mar '22

16 Mar '22

Hi, Alexander, that ok, up to you. I'm not looking into the strings/ctype*c code. On Mar 14, Alexander Barkov wrote: > revision-id: 6a3201d3769 (mariadb-10.6.1-336-g6a3201d3769) > parent(s): 49ecf935415 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-02-28 14:05:08 +0400 > message: > > MDEV-27265 Improve contraction performance in UCA collations > > Adding a hash table for contractions. > > The old code iterated through all items in MY_CONTRACTIONS, > and was much slower, especially for those contractions > in the end of the list. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] c67789f63c8: MDEV-27009 Add UCA-14.0.0 collations - adding version aware implicit weight handling
by Sergei Golubchik 14 Mar '22

14 Mar '22

Hi, Alexander, this is ok too, thanks On Mar 14, Alexander Barkov wrote: > revision-id: c67789f63c8 (mariadb-10.6.1-334-gc67789f63c8) > parent(s): c1010fdfac2 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-02-28 10:31:31 +0400 > message: > > MDEV-27009 Add UCA-14.0.0 collations - adding version aware implicit weight handling > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] c1010fdfac2: MDEV-27009 Add UCA-14.0.0 collations - dump logical positions and contractions
by Sergei Golubchik 14 Mar '22

14 Mar '22

Hi, Alexander, this is ok too On Mar 14, Alexander Barkov wrote: > revision-id: c1010fdfac2 (mariadb-10.6.1-333-gc1010fdfac2) > parent(s): ab62059bbd4 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-02-28 10:31:31 +0400 > message: > > MDEV-27009 Add UCA-14.0.0 collations - dump logical positions and contractions > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] ab62059bbd4: MDEV-27009 Add UCA-14.0.0 collations - Adding implicit weight handling for Unicode-14.0.0
by Sergei Golubchik 14 Mar '22

14 Mar '22

Hi, Alexander, I believe it's ok too, but see a couple of questions below On Mar 14, Alexander Barkov wrote: > revision-id: ab62059bbd4 (mariadb-10.6.1-332-gab62059bbd4) > parent(s): 5fe33342399 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-02-28 10:31:06 +0400 > message: > > MDEV-27009 Add UCA-14.0.0 collations - Adding implicit weight handling for Unicode-14.0.0 > > diff --git a/strings/ctype-uca.c b/strings/ctype-uca.c > index ad0b46ea296..eb6bc2eb9e6 100644 > --- a/strings/ctype-uca.c > +++ b/strings/ctype-uca.c > @@ -31694,7 +31694,7 @@ static inline void > my_uca_implicit_weight_put(uint16 *to, my_wc_t code, uint level) > { > MY_UCA_IMPLICIT_WEIGHT weight; > - weight= my_uca_520_implicit_weight_on_level(code, level); > + weight= my_uca_implicit_weight_on_level(520, code, level); will the version always be a constant in every invocation of my_uca_implicit_weight_put() ? > to[0]= weight.weight[0]; > to[1]= weight.weight[1]; > to[2]= 0; > diff --git a/strings/ctype-uca.h b/strings/ctype-uca.h > index 01f655b06cb..d439b8c578c 100644 > --- a/strings/ctype-uca.h > +++ b/strings/ctype-uca.h > @@ -136,11 +149,11 @@ my_uca_implicit_weight_quaternary() > > > static inline MY_UCA_IMPLICIT_WEIGHT > -my_uca_520_implicit_weight_on_level(my_wc_t code, uint level) > +my_uca_implicit_weight_on_level(uint version, my_wc_t code, uint level) > { > switch (level) { > case 0: > - return my_uca_520_implicit_weight_primary(code); > + return my_uca_implicit_weight_primary(version, code); > case 1: > return my_uca_implicit_weight_secondary(); > case 2: do you mean that secondary/tertiary/etc weights do not depend on the version? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 5fe33342399: MDEV-27009 Add UCA-14.0.0 collations - adding uca-dump into build targets
by Sergei Golubchik 14 Mar '22

14 Mar '22

Hi, Alexander, this is ok (not ok to push as is just yet, please wait until we agree on commits in the branch) On Mar 14, Alexander Barkov wrote: > revision-id: 5fe33342399 (mariadb-10.6.1-331-g5fe33342399) > parent(s): 2467eb2d314 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-02-10 11:53:47 +0400 > message: > > MDEV-27009 Add UCA-14.0.0 collations - adding uca-dump into build targets > > - Adding uca-dump into build targets > - Adding ctype-uca.h and moving implicit weight related routines there > - Reusing implicit weight routines in ctype-uca.c and uca-dump.c > - Adding handling of command line arguments to uca-dump > - Fixing some compile-time warnings in uca-dump.c > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 61b72ed3dde: MDEV-27712 Reduce the size of Lex_length_and_dec_st from 16 to 8
by Sergei Golubchik 11 Mar '22

11 Mar '22

Hi, Alexander, a question about error codes: On Mar 11, Alexander Barkov wrote: > commit 61b72ed3dde > Author: Alexander Barkov <bar(a)mariadb.com> > Date: Wed Feb 9 17:59:38 2022 +0400 > > diff --git a/mysql-test/main/type_blob.test b/mysql-test/main/type_blob.test > index c61ed124139..a5410244ed1 100644 > --- a/mysql-test/main/type_blob.test > +++ b/mysql-test/main/type_blob.test > @@ -517,7 +517,7 @@ CREATE TABLE b15776 (a char(2147483648)); > --error ER_TOO_BIG_FIELDLENGTH > CREATE TABLE b15776 (a char(4294967295)); > # Even BLOB won't hold > ---error ER_TOO_BIG_FIELDLENGTH > +--error ER_NOT_SUPPORTED_YET Why did you change the error? > CREATE TABLE b15776 (a char(4294967296)); > > > diff --git a/mysql-test/main/type_float.result b/mysql-test/main/type_float.result > index 5137a8229b6..9e216711ddd 100644 > --- a/mysql-test/main/type_float.result > +++ b/mysql-test/main/type_float.result > @@ -1162,3 +1162,36 @@ fdec 123.456.789,12345678900000000000000000000000000000 > # > # End of 10.4 tests > # > +# > +# Start of 10.9 tests > +# > +# > +# MDEV-27712 Reduce the size of Lex_length_and_dec_st from 16 to 8 > +# > +CREATE TABLE t1 (a DOUBLE(1000,1000)); > +ERROR 42000: This version of MariaDB doesn't yet support 'scale>255' > +CREATE TABLE t1 (a DOUBLE(1000,0)); > +ERROR 42000: Display width out of range for 'a' (max = 255) > +CREATE TABLE t1 (a DOUBLE(0,1000)); > +ERROR 42000: This version of MariaDB doesn't yet support 'scale>255' > +CREATE TABLE t1 (a DOUBLE(2147483647,2147483647)); > +ERROR 42000: This version of MariaDB doesn't yet support 'scale>255' > +CREATE TABLE t1 (a DOUBLE(2147483647,0)); > +ERROR 42000: Display width out of range for 'a' (max = 255) > +CREATE TABLE t1 (a DOUBLE(0,2147483647)); > +ERROR 42000: This version of MariaDB doesn't yet support 'scale>255' > +CREATE TABLE t1 (a DOUBLE(2147483648,2147483648)); > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '2147483648,2147483648))' at line 1 This isn't technically a *syntax* error Can you use a different error here? > +CREATE TABLE t1 (a DOUBLE(2147483648,0)); > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '2147483648,0))' at line 1 > +CREATE TABLE t1 (a DOUBLE(0,2147483648)); > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '2147483648))' at line 1 > +CREATE TABLE t1 (a DOUBLE(999999999999999999999999999999,999999999999999999999999999999)); > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '999999999999999999999999999999,999999999999999999999999999999))' at line 1 > +CREATE TABLE t1 (a DOUBLE(999999999999999999999999999999,0)); > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '999999999999999999999999999999,0))' at line 1 > +CREATE TABLE t1 (a DOUBLE(0,999999999999999999999999999999)); > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '999999999999999999999999999999))' at line 1 > +# > +# End of 10.9 tests > +# > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index 5bb03544762..fdc54f7df27 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -5975,11 +5977,10 @@ field_type_numeric: > | FLOAT_SYM float_options last_field_options > { > $$.set(&type_handler_float, $2); > - if ($2.length() && !$2.dec()) > + if ($2.has_explicit_length() && !$2.has_explicit_dec()) > { > - int err; > - ulonglong tmp_length= my_strtoll10($2.length(), NULL, &err); > - if (unlikely(err || tmp_length > PRECISION_FOR_DOUBLE)) > + ulonglong tmp_length= $2.length(); you don't really need tmp_length here anymore > + if (unlikely(tmp_length > PRECISION_FOR_DOUBLE)) > my_yyabort_error((ER_WRONG_FIELD_SPEC, MYF(0), > Lex->last_field->field_name.str)); > if (tmp_length > PRECISION_FOR_FLOAT) Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 0402f6dcb67: MDEV-25477 Auto-create breaks replication when triggering event was not replicated
by Sergei Golubchik 07 Mar '22

07 Mar '22

Hi, Aleksey, This was quite good, thanks! Many cases covered, not just ROLLBACK, but also LIMIT 0 and impossible WHERE. See a couple of comments below. Nothing big. On Feb 12, Aleksey Midenkov wrote: > commit 0402f6dcb67 > Author: Aleksey Midenkov <midenok(a)gmail.com> > Date: Tue Feb 8 22:54:03 2022 +0300 > > diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc > index 0403d6e73c3..06068290247 100644 > --- a/sql/sql_delete.cc > +++ b/sql/sql_delete.cc > @@ -507,6 +508,18 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, > if (thd->lex->describe || thd->lex->analyze_stmt) > goto produce_explain_and_leave; > > + if (thd->log_current_statement) > + { > + thd->reset_unsafe_warnings(); > + if (thd->binlog_query(THD::STMT_QUERY_TYPE, > + thd->query(), thd->query_length(), > + transactional_table, FALSE, FALSE, 0) > 0) > + { > + my_error(ER_ERROR_ON_WRITE, MYF(0), "binary log", -1); > + DBUG_RETURN(1); > + } > + } perhaps, move this block into a function? It's repeated four times here. by the way, you don't seem to have a test for a zero-rows insert. That is INSERT ... SELECT with an impossible where or limit 0. In fact, you can have a zero-row insert with a normal insert INSERT t1 VALUES (1); if this (1) is a unique key violation. I believe your code handles this case, thought, just a test case is missing. > + > my_ok(thd, 0); > DBUG_RETURN(0); > } > @@ -932,8 +958,8 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, > else > errcode= query_error_code(thd, killed_status == NOT_KILLED); > > - ScopedStatementReplication scoped_stmt_rpl( > - table->versioned(VERS_TRX_ID) ? thd : NULL); > + StatementBinlog stmt_binlog(thd, table->versioned(VERS_TRX_ID) || > + thd->binlog_need_stmt_format(transactional_table)); I know that's not part of this patch, but still. Just trying to understand. Why did it change to statement-based if VERS_TRX_ID? > /* > [binlog]: If 'handler::delete_all_rows()' was called and the > storage engine does not inject the rows itself, we replicate Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] 880e92a48ba: MDEV-27760 event may non stop replicate in circular semisync setup
by Sergei Golubchik 07 Mar '22

07 Mar '22

Hi, Andrei, I don't see a point in caching the value in memory that is very cheap to calculate per event. I've pushed f8ec4dd38d0 to 10.4-serg three hours ago and Jenkins said it's fine. So I'd suggest we go with it, because we really have to close this blocker and release ES now. And MDEV-27837 will be fixed as a separate bug. queue_event checks global_system_variables.server_id in multiple places, moving only do_accept_own_server_id into Master_info will not make it any better or worse against MDEV-27837. > > As far as I can see, you can calculate it for every event just the same. > > The per event computation is extraneous and not consistent with the > nature of the flag that belongs to the group of events. > > Also notice (or remember) a "FR" > MDEV-27837 disallow `set @@session.server_id` within transaction > so until this bug is fixed it's not 'just the same'. > > I am recomming the patch, assuming that you'll be fine with the Gtid time > only flag computation. > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0