- developers - lists.mariadb.org

Re: [Maria-developers] 394bb653b0b: MDEV-26695: Number of an invalid row is not calculated for table value constructor
by Sergei Golubchik 11 Jan '22

11 Jan '22

Hi, Rucha! please, see below On Jan 11, Rucha Deodhar wrote: > revision-id: 394bb653b0b (mariadb-10.6.1-140-g394bb653b0b) > parent(s): 3c857a07d92 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-10-20 01:30:04 +0530 > message: > > MDEV-26695: Number of an invalid row is not calculated for table value > constructor > > Analysis: counter does not increment while sending rows for table value > constructor and so row_number assumes the … [View More]

1 0

Re: [Maria-developers] ce69e0c8e78: MDEV-24920: Merge "old" SQL variable to "old_mode" sql variable
by Sergei Golubchik 10 Jan '22

10 Jan '22

Hi, Rucha! First, note that it's now a 10.9 task, so don't push it into 10.8, please. As for the patch - code-wise it's all good. New mode names are confusing and should be clarified, see the comment below. On Jan 10, Rucha Deodhar wrote: > revision-id: ce69e0c8e78 (mariadb-10.6.1-88-gce69e0c8e78) > parent(s): 22556499397 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-10-06 13:25:29 +0530 > message: > > MDEV-24920: Merge "old" SQL variable to "… [View More]old_mode" sql variable > > Analysis: There are 2 server variables- "old_mode" and "old". "old" is > no longer needed as "old_mode" has replaced it (however still used in > some places in the code). "old_mode" and "old" has same purpose- > emulate behavior from previous MariaDB versions. So they can be merged > to avoid confusion. Fix: Deprecate "old" variable and create another > mode for @@old_mode to mimic behavior of previous "old" variable. > Create specific modes for specifix task that --old sql variable was > doing earlier and use the new modes instead. > > diff --git a/mysql-test/suite/sys_vars/r/old_mode_basic.result b/mysql-test/suite/sys_vars/r/old_mode_basic.result > index a6b95f1c60c..2f3742160eb 100644 > --- a/mysql-test/suite/sys_vars/r/old_mode_basic.result > +++ b/mysql-test/suite/sys_vars/r/old_mode_basic.result > @@ -264,3 +264,42 @@ SET @@collation_database = @save_collation_database; > # > # End of 10.6 test > # > +# > +# Beginning of 10.7 test > +# 10.9 now > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index c2a219ee015..d1eaacd777c 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -3755,6 +3774,8 @@ static const char *old_mode_names[]= > "NO_PROGRESS_INFO", > "ZERO_DATE_TIME_CAST", > "UTF8_IS_UTF8MB3", > + "INDEX_MASKING", > + "CHECKSUM_FORMATTING", that's confusing. I don't understsand from the name of the option what "CHECKSUM_FORMATTING" or "INDEX_MASKING" changes. "NO_PROGRESS_INFO" is clear, there won't be progress info "UTF8_IS_UTF8MB3" is clear "ZERO_DATE_TIME_CAST" is kind of understandable too, CAST(<time> AS DATETIME) will be "0000-00-00 <time>", that is zero date CHECKSUM_FORMATTING and INDEX_MASKING is unclear. For CHECKSUM_FORMATTING I'd suggest CHECKSUM_SLOW_NULLS (see the commit that introduced this behavior: 496741d5761f) "INDEX_MASKING" was introduced in the commit 79542930ea1c, see the comment there, it explains what was done. This should help you to suggest a better name for this mode. > 0 > }; > > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org [View Less]

1 0

Re: [Maria-developers] 9ea85a70a75: MDEV-24654 GTID event falsely marked transactional
by Sergei Golubchik 10 Jan '22

10 Jan '22

Hi, Andrei! On Jan 10, Andrei Elkin wrote: > revision-id: 9ea85a70a75 (mariadb-10.2.40-4-g9ea85a70a75) > parent(s): 160d97a4aaa > author: Andrei Elkin > committer: Andrei Elkin > timestamp: 2021-08-08 14:20:57 +0300 > message: > > MDEV-24654 GTID event falsely marked transactional > > GTID event can be falsely marked transactional in few cases including > binary-logging on the slave side upon execution. > In some execution branches bin-logging of non-… [View More]

1 0

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

10 Jan '22

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

2 3

Re: [Maria-developers] ce507903d0c: MDEV-22742 UBSAN: Many overflow issues in strings/decimal.c - runtime error: signed integer overflow: x * y cannot be represented in type 'long long int' (on optimized builds).
by Sergei Golubchik 09 Jan '22

09 Jan '22

Hi, Alexey! On Jan 09, Alexey Botchkov wrote: > revision-id: ce507903d0c (mariadb-10.2.40-161-gce507903d0c) > parent(s): 0dae41637ab > author: Alexey Botchkov > committer: Alexey Botchkov > timestamp: 2021-11-22 09:58:46 +0400 > message: > > MDEV-22742 UBSAN: Many overflow issues in strings/decimal.c - runtime error: signed integer overflow: x * y cannot be represented in type 'long long int' (on optimized builds). > > Avoid integer overflow, do the check before … [View More]

1 0

Re: [Maria-developers] 0dfe5a31573: MDEV-22441 implement a generic way to change a value of a variable in a scope
by Sergei Golubchik 09 Jan '22

09 Jan '22

Hi, Eugene! On Jan 09, Eugene Kosov wrote: > revision-id: 0dfe5a31573 (mariadb-10.7.1-13-g0dfe5a31573) > parent(s): 1e54a9716d2 > author: Eugene Kosov > committer: Eugene Kosov > timestamp: 2021-12-03 16:12:27 +0600 > message: > > MDEV-22441 implement a generic way to change a value of a variable in a scope > > Example: > { > auto _= make_scope_value(var, tmp_value); > } > > make_scope_value(): a function which returns RAII object which … [View More]

1 0

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

06 Jan '22

Hi, Aleksey! On Oct 17, Aleksey Midenkov wrote: > commit 8a84f5a40c1 > Author: Aleksey Midenkov <midenok(a)gmail.com> > Date: Thu May 27 17:00:14 2021 +0300 > > MDEV-24176 Preparations > > 1. moved fix_vcol_exprs() call to open_table() > > mysql_alter_table() doesn't do lock_tables() so it cannot win from > fix_vcol_exprs() from there. Tests affected: main.default_session This is likely wrong, but the old code was wrong too. Neither … [View More]

2 5

Re: [Maria-developers] 3617add6975: MDEV-27208: Extend CRC32() and implement CRC32C()
by Sergei Golubchik 05 Jan '22

05 Jan '22

Hi, Marko! On Jan 03, Marko Mäkelä wrote: > revision-id: 3617add6975 (mariadb-10.6.1-252-g3617add6975) > parent(s): bc90f5d6967 > author: Marko Mäkelä > committer: Sergei Golubchik > timestamp: 2021-12-15 19:54:34 +0100 > message: > > MDEV-27208: Extend CRC32() and implement CRC32C() > > We used to define a native unary function crc32() that computes the CRC-32 > of a string using the ISO 3309 polynomial that is being used by zlib > and many others. … [View More]

2 1

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

31 Dec '21

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

1 0

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

31 Dec '21

Hi, Rucha! On Dec 29, Rucha Deodhar wrote: > On Wed, Dec 29, 2021 at 12:50 AM Sergei Golubchik <serg(a)mariadb.org> wrote: > > > Why did put your fix here in a common code path in select, and not > > in, say, select_insert::send_data() ? > > Yes, the counter can be incremented in select_insert::send_data() too. > > But after writing record to INSERT table, we go back to > evaluate_join_records() where the counter is incremented > unconditionally. … [View More]

1 0