July 2021 - developers - lists.mariadb.org

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

24 Aug '22

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

2 5

Re: [Maria-developers] cdb6db8ce38: MDEV-25444 mysql --binary-mode is not able to replay some mysqlbinlog outputs
by Sergei Golubchik 14 Aug '21

14 Aug '21

Hi, Sachin! On Jul 12, Sachin Kumar wrote: > revision-id: cdb6db8ce38 (mariadb-10.3.26-155-gcdb6db8ce38) > parent(s): 72753d2b65f > author: Sachin Kumar <sachin.setiya(a)mariadb.com> > committer: Sachin Kumar <sachin.setiya(a)mariadb.com> > timestamp: 2021-05-19 15:46:57 +0100 > message: > > MDEV-25444 mysql --binary-mode is not able to replay some mysqlbinlog outputs > > Problem:- Some binary data is inserted into the table using Jconnector. When > binlog dump of the data is applied using mysql cleint it gives syntax error. > > Reason:- > After investigating it turns out to be a issue of mysql client not able to properly > handle \\\0 <0 in binary>. In all binary files where mysql client fails to insert > these 2 bytes are commom (0x5c00) > > Solution:- > I have changed mysql.cc to include for the possibility that binary string can > have \\\0 in it. > > --- > client/mysql.cc | 8 ++++++-- > mysql-test/main/binary_zero_insert.result | 6 ++++++ > mysql-test/main/binary_zero_insert.test | 15 +++++++++++++++ > mysql-test/std_data/binary_zero_insert.bin | Bin 0 -> 87 bytes > 4 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/client/mysql.cc b/client/mysql.cc > index 2a7c4eaf3e5..8530c105820 100644 > --- a/client/mysql.cc > +++ b/client/mysql.cc > @@ -2319,8 +2319,12 @@ static bool add_line(String &buffer, char *line, size_t line_length, > { > // Found possbile one character command like \c > > + inchar = (uchar) *++pos; > + // In Binary mode , when in_string is not null \0 should not be treated as > + // end statement. This can happen when we are in middle of binary data which > + // can contain \0 and its quoted with ' '. > + if (!real_binary_mode && !*in_string && !inchar) Two comments here. 1. if you check the history where this "real_binary_mode" came from, you'd arrive at commit 2db4392bf4cb and the test file mysql_binary_mode.test So, I'd say it make sense to add your test there too. And in the same style, wthout a separate binary file. in my experimening I did it as simple as --- a/mysql-test/main/mysql_binary_mode.test +++ b/mysql-test/main/mysql_binary_mode.test @@ -16,14 +16,14 @@ let $table_name_right= `SELECT 0x410D0A42`; let $table_name_wrong= `SELECT 0x410A42`; # 0x410042 => 'A\0B' -let $char0= `SELECT 0x410042`; +let $char0= `SELECT 0x4100425C0043`; (there already was a test for a \0 byte, but not for an escaped \\\0) but if you'd like you can add a separate test with a header and everything. > - if (!(inchar = (uchar) *++pos)) > - break; // readline adds one '\' 2. What does the old code mean? Why returning if inchar==0 ? What does the comment mean? It's the code from the last millenium, does it even apply now? For example, there can be no \0 bytes in the string, because of if (!real_binary_mode && strlen(line) != line_length) ... in other words, shouldn't we just remove the if? I tried, at least the main suite didn't break. > + break; // readline adds one '\' > if (*in_string || inchar == 'N') // \N is short for NULL > { // Don't allow commands in string > *out++='\\'; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] 203aa38f9b6: MDEV-26102: gcov 9.1 doesn't support intermediate format any more, but json instead - refactoring needed
by Sergei Golubchik 29 Jul '21

29 Jul '21

Hi, Anel! On Jul 28, Anel Husakovic wrote: > revision-id: 203aa38f9b6 (mariadb-10.2.39-96-g203aa38f9b6) > parent(s): afe00bb7cce > author: Anel Husakovic > committer: Anel Husakovic > timestamp: 2021-07-27 16:09:41 +0200 > message: > > MDEV-26102: gcov 9.1 doesn't support intermediate format any more, but json instead - refactoring needed I've renamed the MDEV to have a shorter title, use the new one in your next commit > > Reviewed by: serg(a)mariadb.com that's not right, I haven't reviewed it yet :) > diff --git a/mysql-test/dgcov.pl b/mysql-test/dgcov.pl > index 2c00c64d1ff..e2aadec33e0 100755 > --- a/mysql-test/dgcov.pl > +++ b/mysql-test/dgcov.pl > @@ -25,6 +25,8 @@ use warnings; > use Getopt::Long; > use File::Find; > use Cwd qw/realpath/; > +use IO::Uncompress::Gunzip qw(gunzip $GunzipError); > +use JSON::PP; move this down, dcgov should not try to load these modules for for gcc < 9 > my $opt_verbose=0; > my $opt_generate; > @@ -32,6 +34,7 @@ my $opt_help; > my $opt_purge; > my $opt_only_gcov; > my $opt_skip_gcov; > +my $fname; and this > > my %cov; > my $file_no=0; > @@ -62,12 +65,15 @@ my $res; > my $cmd; > if ($opt_purge) > { > - $cmd= "find . -name '*.da' -o -name '*.gcda' -o -name '*.gcov' -o ". > + $cmd= "find . -name '*.da' -o -name '*.gcda*' -o -name '*.gcov' -o ". > "-name '*.dgcov' | grep -v 'README\.gcov' | xargs rm -f ''"; > logv "Running: $cmd"; > system($cmd)==0 or die "system($cmd): $? $!"; > exit 0; > } > + > +my $gcc_version= `gcc -dumpversion`; > +$gcc_version=~ s/(\d).*$/$1/; you don't need this second line, I suspect > > find(\&gcov_one_file, $root); > find(\&write_coverage, $root) if $opt_generate; > @@ -162,6 +168,7 @@ sub gcov_one_file { > } > > # now, read the generated file > + if ($gcc_version <9){ > for my $gcov_file (<$_*.gcov>) { > open FH, '<', "$gcov_file" or die "open(<$gcov_file): $!"; > my $fname; > @@ -183,6 +189,18 @@ sub gcov_one_file { > $cov{$fname}->{$1}+=$2; > } > close(FH); > + } > + } > + else { put both `use` lines here > + my $gcov_file_json; > + gunzip "$_.gcov.json.gz" => \$gcov_file_json or die "gunzip($_.gcov.json.gz): $GunzipError"; > + my $obj= decode_json $gcov_file_json; > + for my $file (@{$obj->{files}}) { > + $fname= $file->{file}; and write `my $fname` here > + for my $line (@{$file->{lines}}){ > + $cov{$fname}->{$line->{line_number}}= $line->{count}; shouldn't it be += $line->{count} ? The point is to accumulate all counters, see how it's done for non-json intermediate format. > + } > + } > } > } > > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] ce289840131: MDEV-24511 null field is created with CREATE..SELECT
by Nikita Malyavin 28 Jul '21

28 Jul '21

Hello, Sergei! Looks fine for me. So if you both prefer it more, let's use this variant. Regards, Nikita On Wed, Jul 28, 2021 at 12:08 PM Sergei Golubchik <serg(a)mariadb.org> wrote: > Hi, Nikita! > > On Jul 27, Nikita Malyavin wrote: > > revision-id: ce289840131 (mariadb-10.3.30-31-gce289840131) > > parent(s): b50ea900636 > > author: Nikita Malyavin > > committer: Nikita Malyavin > > timestamp: 2021-07-22 22:38:31 +0300 > > message: > > > > MDEV-24511 null field is created with CREATE..SELECT > > > > The regression has been initially appeared by MDEV-12588 patch > (441349aa). > > > > The reason CREATE... SELECT NULL does not create null field is that > > it calls handle_select(), where the field is created directly from Item, > > by calling type_handler->type_handler_for_tmp_table(), instead of plain > > type_handler. > > > > In UNION case, we'll already have Item_field's containing UNION'ed tmp > > table fields to the moment of select_create::prepare() call. > > > > There, item's type_handler is used directly in > > st_select_lex_unit::join_union_item_types(), instead of > > type_handler_for_tmp_table() or type_handler_for_union(). > > > > Using those two involve too many changes in CREATE...SELECT...UNION > > behavior, which may be reasonably undesired to be pushed in 10.3 > > > > However, patching Field_null::type_handler() directly seems to be > accurate > > enough. > > > > diff --git a/mysql-test/main/union.result b/mysql-test/main/union.result > > index a892f6c9e40..b19427948a6 100644 > > --- a/mysql-test/main/union.result > > +++ b/mysql-test/main/union.result > > @@ -1609,7 +1609,7 @@ NULL binary(0) YES NULL > > CREATE TABLE t5 SELECT NULL UNION SELECT NULL; > > DESC t5; > > Field Type Null Key Default Extra > > -NULL null YES NULL > > +NULL binary(0) YES NULL > > CREATE TABLE t6 > > SELECT * FROM (SELECT * FROM (SELECT NULL)a) b UNION SELECT a FROM t1; > > DESC t6; > > diff --git a/sql/field.h b/sql/field.h > > index a3385736c0a..2af5346bea9 100644 > > --- a/sql/field.h > > +++ b/sql/field.h > > @@ -2526,7 +2526,7 @@ class Field_null :public Field_str { > > :Field_str(ptr_arg, len_arg, null, 1, > > unireg_check_arg, field_name_arg, collation) > > {} > > - const Type_handler *type_handler() const { return &type_handler_null; > } > > + const Type_handler *type_handler() const { return > &type_handler_string; } > > Information_schema_character_attributes > > information_schema_character_attributes() const > > { > > This looks suspiciously risky to me. The problem was specifically in > UNION, but you change the very basic idea of what type the Field_null > is. > > I cannot clearly see what else might this possibly affect, field's type > is used everywhere. > > It would be safer and more logical to fix specifically field creation > for UNION. > > I've discussed it with Bar, who implemented this whole Type_handler > hierarchy, he suggested to use Type_handler::union_element_finalize() > for that. I've attached the patch that does that. > > union_element_finalize() is for 10.4, so I also backported it to 10.3, > it's very straightforward though. > > What do you think? > > Regards, > Sergei > VP of MariaDB Server Engineering > and security(a)mariadb.org > -- Nikita Malyavin, Software Engineer MariaDB Corporation

1 0

Re: [Maria-developers] ce289840131: MDEV-24511 null field is created with CREATE..SELECT
by Sergei Golubchik 28 Jul '21

28 Jul '21

Hi, Nikita! On Jul 27, Nikita Malyavin wrote: > revision-id: ce289840131 (mariadb-10.3.30-31-gce289840131) > parent(s): b50ea900636 > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2021-07-22 22:38:31 +0300 > message: > > MDEV-24511 null field is created with CREATE..SELECT > > The regression has been initially appeared by MDEV-12588 patch (441349aa). > > The reason CREATE... SELECT NULL does not create null field is that > it calls handle_select(), where the field is created directly from Item, > by calling type_handler->type_handler_for_tmp_table(), instead of plain > type_handler. > > In UNION case, we'll already have Item_field's containing UNION'ed tmp > table fields to the moment of select_create::prepare() call. > > There, item's type_handler is used directly in > st_select_lex_unit::join_union_item_types(), instead of > type_handler_for_tmp_table() or type_handler_for_union(). > > Using those two involve too many changes in CREATE...SELECT...UNION > behavior, which may be reasonably undesired to be pushed in 10.3 > > However, patching Field_null::type_handler() directly seems to be accurate > enough. > > diff --git a/mysql-test/main/union.result b/mysql-test/main/union.result > index a892f6c9e40..b19427948a6 100644 > --- a/mysql-test/main/union.result > +++ b/mysql-test/main/union.result > @@ -1609,7 +1609,7 @@ NULL binary(0) YES NULL > CREATE TABLE t5 SELECT NULL UNION SELECT NULL; > DESC t5; > Field Type Null Key Default Extra > -NULL null YES NULL > +NULL binary(0) YES NULL > CREATE TABLE t6 > SELECT * FROM (SELECT * FROM (SELECT NULL)a) b UNION SELECT a FROM t1; > DESC t6; > diff --git a/sql/field.h b/sql/field.h > index a3385736c0a..2af5346bea9 100644 > --- a/sql/field.h > +++ b/sql/field.h > @@ -2526,7 +2526,7 @@ class Field_null :public Field_str { > :Field_str(ptr_arg, len_arg, null, 1, > unireg_check_arg, field_name_arg, collation) > {} > - const Type_handler *type_handler() const { return &type_handler_null; } > + const Type_handler *type_handler() const { return &type_handler_string; } > Information_schema_character_attributes > information_schema_character_attributes() const > { This looks suspiciously risky to me. The problem was specifically in UNION, but you change the very basic idea of what type the Field_null is. I cannot clearly see what else might this possibly affect, field's type is used everywhere. It would be safer and more logical to fix specifically field creation for UNION. I've discussed it with Bar, who implemented this whole Type_handler hierarchy, he suggested to use Type_handler::union_element_finalize() for that. I've attached the patch that does that. union_element_finalize() is for 10.4, so I also backported it to 10.3, it's very straightforward though. What do you think? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] c34533186ab: MDEV-18734 ASAN heap-use-after-free upon sorting by blob column from partitioned table
by Sergei Golubchik 26 Jul '21

26 Jul '21

Hi, Aleksey! As before, despite the email subject, it's git diff dfa2d0bc13..e533b5fb307 Don't be confused :) On Jul 21, Aleksey Midenkov wrote: > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc > index c53732a2b51..70ed14448ea 100644 > --- a/sql/ha_partition.cc > +++ b/sql/ha_partition.cc > @@ -5130,7 +5135,18 @@ bool ha_partition::init_record_priority_queue() > i= bitmap_get_next_set(&m_part_info->read_partitions, i)) > { > DBUG_PRINT("info", ("init rec-buf for part %u", i)); > - int2store(ptr, i); > + if (table->s->blob_fields) > + { > + for (uint j= 0; j < table->s->blob_fields; ++j, ++objs) > + { > + blob_storage[j]= new (objs) Ordered_blob_storage; > + if (!blob_storage[j]) > + DBUG_RETURN(true); is this possible? > + } > + *((Ordered_blob_storage ***) ptr)= blob_storage; > + blob_storage+= table->s->blob_fields; > + } > + int2store(ptr + sizeof(String **), i); > ptr+= m_priority_queue_rec_len; > } > m_start_key.key= (const uchar*)ptr; > @@ -6291,6 +6333,43 @@ int ha_partition::handle_ordered_index_scan_key_not_found() > } > > > +void ha_partition::swap_blobs(uchar * rec_buf, Ordered_blob_storage ** storage, bool restore) > +{ > + uint *ptr, *end; > + uint blob_n= 0; > + table->move_fields(table->field, rec_buf, table->record[0]); > + for (ptr= table->s->blob_field, end= ptr + table->s->blob_fields; > + ptr != end; ++ptr, ++blob_n) > + { > + DBUG_ASSERT(*ptr < table->s->fields); > + Field_blob *blob= (Field_blob*) table->field[*ptr]; > + DBUG_ASSERT(blob->flags & BLOB_FLAG); > + DBUG_ASSERT(blob->field_index == *ptr); > + if (!bitmap_is_set(table->read_set, *ptr) || blob->is_null()) > + continue; > + > + Ordered_blob_storage &s= *storage[blob_n]; > + > + if (restore) > + { > + if (!s.blob.is_empty()) > + blob->swap(s.blob, s.set_read_value); don't you need here something like else blob->reset(); to make blob empty? > + } > + else > + { > + bool set_read_value; > + String *cached= blob->cached(set_read_value); > + if (cached) > + { > + cached->swap(s.blob); > + s.set_read_value= set_read_value; > + } > + } > + } > + table->move_fields(table->field, table->record[0], rec_buf); On swap_blobs() you always move all fields from rec_buf to table->record[0]? Why? This looks very suspicious. > +} > + > + > /* > Common routine to handle index_next with ordered results > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] c4835af7e8c: MDEV-16026: Forbid global system_versioning_asof in non-default time zone
by Sergei Golubchik 26 Jul '21

26 Jul '21

Hi, Nikita! I suggest to squash commits from c4835af7e8c to 80e3007a5a1 into one. It'll be simpler, no intermediate juggling with MYSQL_TIME anymore. This squashed commit is ok to push (make sure to mention both MDEVs in the commit comment, but not necessarily in the first line) On Jul 26, Nikita Malyavin wrote: > revision-id: c4835af7e8c (mariadb-10.3.30-30-gc4835af7e8c) > parent(s): ec3289b6de4 > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2021-07-21 00:21:31 +0300 > message: > > MDEV-16026: Forbid global system_versioning_asof in non-default time zone > > * store `system_versioning_asof` in unix time; > * both session and global vars are processed in session timezone; > * setting `default` does not copy global varibale abymore. Instead, it sets > system_time to SYSTEM_TIME_UNSPECIFIED, which means that no 'AS OF' time > is applied and `now()` can be assumed > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] a4a5c2d5963: MDEV-25908: -e does not work for my_print_defaults
by Sergei Golubchik 26 Jul '21

26 Jul '21

Hi, Rucha! On Jul 26, Rucha Deodhar wrote: > revision-id: a4a5c2d5963 (mariadb-10.5.11-52-ga4a5c2d5963) > parent(s): 675b72d0da1 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-07-23 19:51:57 +0530 > message: > > MDEV-25908: -e does not work for my_print_defaults > > Analysis: get_defaults_options() rewrites the value for my_defaults_file, > my_defaults_extra_file and my_defaults_group_suffix to 0. So the config file > can't be read. > Fix: Let handle_options() handle --defaults* option and create a new command > line to pass it to load_defaults(). > > diff --git a/libmariadb b/libmariadb > index 802ce584a26..180c543704d 160000 > --- a/libmariadb > +++ b/libmariadb > @@ -1 +1 @@ > -Subproject commit 802ce584a26fdc0ba67fcf35e277bf3c7440956a > +Subproject commit 180c543704d627a50a52aaf60e24ca14e0ec4686 ^^^^^ This is wrong, please, remove it from your commit Otherwise ok to push Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] Accessing read records during join for condition pushdown
by Berrocal, Eduardo 23 Jul '21

23 Jul '21

Hello list (first email here, hopefully sending it to the right address), I have been trying to solve this problem for a while now, and I am stuck. I wonder if what I am trying to do is even possible. I am implementing a new storage engine which handles engine condition pushdown. Now, when MariaDB is working on a JOIN using more than one table (for simplicity, let's assume 2), how can I access the read records for table 1 when working on a condition pushdown for table 2? When joining like this: table1.field1 = table2.field1, I can see 2 Item objects of type FIELD in the condition. However, I can't find where the read records for table1 are so I can effectively filter records in table2 by those values. Thanks a lot for your help, [A close up of a sign Description automatically generated] Eduardo Berrocal García de Carellán Senior Software Engineer Intel Corporation | intel.com<http://intel.com/>

3 9

Re: [Maria-developers] 412e696fd2b: MDEV-25395 server recovery hits replication event checksum error
by Sergei Golubchik 23 Jul '21

23 Jul '21

Hi, Andrei! fdle changes - ok, I've seen them in 21117. cur_log/etc - clear. What are the changes around prev_event_pos for? On Jul 23, Andrei Elkin wrote: > revision-id: 412e696fd2b (mariadb-10.3.26-136-g412e696fd2b) > parent(s): 2fa526e26e8 > author: Andrei Elkin > committer: Andrei Elkin > timestamp: 2021-04-15 17:11:07 +0300 > message: > > MDEV-25395 server recovery hits replication event checksum error > > When @@global.binlog_checksum is changing from none to > crc32 and only the first of two Binlog_checkpoint_log_event gets written to > the crc32 rotated binlog file before the server crashes, the recovery faces a checksum > verification error. > > The reason was that recovery algorithm did not realize > potentially varying binlog checksum at scanning of > a sequence of binlog files. > > That is corrected now. > The master-checksum-verify server recovery aborts now and stops > the server due to the checksum error. > A simulation test confirms the fixes. > > Induced changes: > > rpl_cant_read_event_incident got affected because the test > adopts an external not properly closed binlog file that contains > events which can't pass checksum verification. > A work-around is done to accept that binlog and evade recovery. > > binlog_recover_checksum_error needed refinement as > the master-checksum-verify configured server is made to stop > due to the checksum error. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0