April 2025 - developers - lists.mariadb.org

Re: 0ed7370d133: MDEV-24726 Assertion `0' failed in Field_varstring_compressed::key_cmp
by Sergei Golubchik 04 Apr '25

04 Apr '25

Hi, Aleksey, Despite the email subject, this is a combined diff of three commits, git diff 071c20fd23b2 2fecd944d6dc On Apr 04, Aleksey Midenkov wrote: > diff --git a/sql/field.cc b/sql/field.cc > index 8b96c473ab6..d6be66cf399 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -2516,7 +2521,7 @@ Field *Field::new_key_field(MEM_ROOT *root, TABLE *new_table, > uchar *new_null_ptr, uint new_null_bit) > { > Field *tmp; > - if ((tmp= make_new_field(root, new_table, table == new_table))) > + if ((tmp= make_new_field({root, new_table, table == new_table}))) it's a creative trick, I admit, but let's not do it. looks very weird, we don't have it anywhere else. it's not needed either, because you don't need to pass down a new argument, see below > { > tmp->ptr= new_ptr; > tmp->null_ptr= new_null_ptr; > @@ -8278,12 +8281,28 @@ int Field_varstring::cmp_binary(const uchar *a_ptr, const uchar *b_ptr, > } > > > -Field *Field_varstring::make_new_field(MEM_ROOT *root, TABLE *new_table, > - bool keep_type) > +Field *Field_varstring::make_new_field(make_new_field_args args) > { > - Field_varstring *res= (Field_varstring*) Field::make_new_field(root, > - new_table, > - keep_type); > + Field_varstring *res= (Field_varstring*) Field::make_new_field(args); > + if (res) > + res->length_bytes= length_bytes; > + return res; > +} > + > + > +Field *Field_varstring_compressed::make_new_field(make_new_field_args args) > +{ > + Field_varstring *res; > + if (args.tmp_field) tmp field is a field of a tmp table. Unfortunately, INTERNAL_TMP_TABLE value of new_table->s->tmp_table is very overloaded and no longer means internal temporary table anymore. I think we'll need to fix it someday. But a smaller fix for today could be, like @@ -1113,6 +1113,11 @@ struct TABLE_SHARE return (tmp_table == SYSTEM_TMP_TABLE) ? 0 : table_map_id; } + bool is_optimizer_tmp_table() + { + return tmp_table == INTERNAL_TMP_TABLE && !db.length && table_name.length; + } + using the fact that optimizer tmp tables are all named "(temporary)" or something like that and have no db. Using this one can do a fix, like @@ -8281,11 +8279,23 @@ int Field_varstring::cmp_binary(const uchar *a_ptr, con> Field *Field_varstring::make_new_field(MEM_ROOT *root, TABLE *new_table, bool keep_type) { - Field_varstring *res= (Field_varstring*) Field::make_new_field(root, + Field_varstring *res; + if (new_table->s->optimizer_tmp_table()) + { + res= new (root) Field_varstring(ptr, field_length, length_bytes, null_ptr, + null_bit, Field::NONE, &field_name, + new_table->s, charset()); + if (res) + res->init_for_make_new_field(new_table, orig_table); + } + else + { + res= (Field_varstring*) Field::make_new_field(root, new_table, keep_type); if (res) res->length_bytes= length_bytes; + } return res; } > + { > + /* Compressed fields can't have keys (see Field_varstring_compressed::key_cmp()). */ > + res= new (args.root) Field_varstring(*this); > + res->init_new_field(args); > + /* See Column_definition::create_length_to_internal_length_string() */ > + res->field_length--; > + } > + else > + res= (Field_varstring*) Field::make_new_field(args); > if (res) > res->length_bytes= length_bytes; > return res; Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

MDEV-36107: Proposal for Enhancements in MariaDB Operator Handling
by Harshit Vijay Kumar 02 Apr '25

02 Apr '25

Dear MariaDB Developer Community, I hope you are doing well. I am *Harshit Vijay Kumar*, a final-year undergraduate student in Computer Science. I have been researching *MDEV-36107 <https://jira.mariadb.org/browse/MDEV-36107>* for *GSoC 2025* and, during my exploration of operator operations in MariaDB, I came across some behaviors that caught my attention. To further investigate, I compared these behaviors with MySQL to understand their differences. *Observations: * *(1) Handling Large Number Multiplications Using Exponents* - Currently, multiplying two large integers results in an *overflow*, returning 0 instead of using an exponent-based representation. SELECT 18014398509481984 * 18014398509481984; *MariaDB Output:* 0 *SQL Output:* 3.2451855365842673e+32 - Interestingly, if one of the operands is a *floating-point number*, the result is computed without overflow: SELECT 18014398509481984 * 18014398509481984399405765456789087654390954.0; *Output:* 324518553658426733978210646499548233026655740664701555572736.0 - This inconsistency between integer-only and mixed-type operations is confusing. A standardized approach, possibly leveraging exponent notation, could improve clarity. *(2) Rounding Floating-Point Outputs* - MariaDB currently returns full precision in floating-point calculations, which can lead to unnecessary precision in practical use cases. SELECT 1.0376728938476536278903984765 + 1.039876453672890938476536278902345678908765434567890; *MariaDB Output:* 2.07754934752054456636693475540234567891 *SQL Output:* 2.077549347520544 - A rounding mechanism would allow for concise representation and reduce storage overhead, especially for users unaware of excessive precision in their datasets. *(3) Eliminating Unnecessary Zeros in Floating-Point Representation* SELECT 1.0000 * 1.0000000000; *MariaDB Output:* 1.00000000000000 - While useful in some cases, the inclusion of excessive trailing zeros unnecessarily increases storage usage. A more compact representation should be considered, where only *significant digits* are retained. *(4) Implementation of == Equality Operator* - Currently, MariaDB allows the *assignment operator (=)* for comparisons. However, since = is primarily used for assignments, this dual-purpose behavior can be confusing. - Proposal: Introduce a dedicated *== equality operator* for clear and intuitive comparisons across *INT, FLOAT, BOOL, and CHAR* types. - Expected behavior: - If values match → return 1 - If values don’t match → return 0 Testing & Next Steps To implement these improvements, I plan to add *test cases* in mysql-test covering all edge cases for these operators. I will follow the approach used in the following implementations: - func_equal.result <https://github.com/MariaDB/server/blob/main/mysql-test/main/func_equal.resu…> - func_equal.test <https://github.com/MariaDB/server/blob/main/mysql-test/main/func_equal.test> I would really appreciate your insights on these ideas. If any of these behaviors have intentional design choices, I would love to understand the rationale behind them. Looking forward to your feedback! Best Regards, Harshit Vijay Kumar

1 0

Re: 0a0c5f349c6: MDEV-36425 Extend read_only to also block share locks and super user
by Sergei Golubchik 02 Apr '25

02 Apr '25

Hi, Monty, On Apr 01, Michael Widenius wrote: > revision-id: 0a0c5f349c6 (mariadb-11.8.1-42-g0a0c5f349c6) > parent(s): 21321025e5e > author: Michael Widenius > committer: Michael Widenius > timestamp: 2025-03-31 20:09:03 +0300 > message: > > MDEV-36425 Extend read_only to also block share locks and super user > > The main purpose of this allow one to use the --read-only > option to ensure that no one can issue a query that can > block replication. > > The --read-only option can now take 4 different values: > 0 No read only (as before). > 1 Blocks changes for users without the 'READ ONLY ADMIN' > privilege (as before). > 2 Blocks in addition LOCK TABLES and SELECT IN SHARE MODE > for not 'READ ONLY ADMIN' users. > 3 Blocks in addition 'READ_ONLY_ADMIN' users for all the > previous statements. > diff --git a/mysql-test/main/read_only.result b/mysql-test/main/read_only.result > index 65cc12ffce9..89390ad65d5 100644 > --- a/mysql-test/main/read_only.result > +++ b/mysql-test/main/read_only.result > @@ -69,11 +69,14 @@ set global read_only=1; > connection con1; > select @@global.read_only; > @@global.read_only > -0 > +OFF > unlock tables ; > +Timeout in wait_condition.inc for SELECT @@global.read_only= 1 > +Id User Host db Command Time State Info Progress > +5 test localhost test Query 0 starting show full processlist 0.000 Forgot to fix? It should wait for @@global.read_only='READ_ONLY' now > select @@global.read_only; > @@global.read_only > -1 > +READ_ONLY > connection default; > reap; > connection default; > diff --git a/sql/lock.cc b/sql/lock.cc > index 1296e5455d5..8f9f97f88b7 100644 > --- a/sql/lock.cc > +++ b/sql/lock.cc > @@ -179,19 +198,48 @@ lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags) > > /* > Prevent modifications to base tables if READ_ONLY is activated. > - In any case, read only does not apply to temporary tables. > + In any case, read only does not apply to temporary tables or slave > + threads. > */ > - if (!(flags & MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY) && !t->s->tmp_table) > + if (unlikely(opt_readonly) && > + !(flags & MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY) && !t->s->tmp_table && > + !thd->slave_thread) > { > - if (t->reginfo.lock_type >= TL_FIRST_WRITE && > - !ignore_read_only && opt_readonly && !thd->slave_thread) > + switch (opt_readonly) // Impossible you have "Impossible" comment on a wrong line > { > - my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); > - DBUG_RETURN(1); > + case READONLY_OFF: > + DBUG_ASSERT(0); > + break; > + case READONLY_ON: // Compatibility > + if (!(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && > + t->reginfo.lock_type >= TL_FIRST_WRITE) > + { > + mariadb_error_read_only(); > + DBUG_RETURN(1); I'd make mariadb_error_read_only() to return 1. Then you can write DBUG_RETURN(mariadb_error_read_only()); here and also in all other places, check it out, it's always followed by return 1. > + } > + break; > + case READONLY_NO_LOCK: > + if (!(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && > + (thd->lex->sql_command == SQLCOM_LOCK_TABLES || > + t->reginfo.lock_type >= TL_FIRST_WRITE || > + t->reginfo.lock_type == TL_READ_WITH_SHARED_LOCKS)) may be t->reginfo.lock_type >= TL_READ_WITH_SHARED_LOCKS ? > + { > + mariadb_error_read_only(); > + DBUG_RETURN(1); > + } > + break; > + case READONLY_NO_LOCK_NO_ADMIN: > + if (thd->lex->sql_command == SQLCOM_LOCK_TABLES || > + t->reginfo.lock_type >= TL_FIRST_WRITE || > + t->reginfo.lock_type == TL_READ_WITH_SHARED_LOCKS) and here > + { > + mariadb_error_read_only(); > + DBUG_RETURN(1); > + } > + break; > } > } > } > - > /* > Locking of system tables is restricted: > locking a mix of system and non-system tables in the same lock > diff --git a/sql/sql_class.h b/sql/sql_class.h > index d65cec37154..34ee8391009 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -3448,9 +3452,12 @@ class THD: public THD_count, /* this must be first */ > */ > inline bool is_read_only_ctx() rename to, for example, check_read_only_with_errror() > { > - return opt_readonly && > - !(security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && > - !slave_thread; > + if (likely(!opt_readonly) || slave_thread || > + ((security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && > + opt_readonly != READONLY_NO_LOCK_NO_ADMIN)) > + return false; > + mariadb_error_read_only(); > + return true; > } > > private: > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index 0247afd91a6..545b4674da1 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -3253,15 +3253,24 @@ static bool fix_read_only(sys_var *self, THD *thd, enum_var_type type) > transition (especially when transitioning from false to true) and > synchronizes both booleans in the end. > */ > -static Sys_var_on_access_global<Sys_var_mybool, > + > +const char *read_only_mode_names[]= > +{"OFF", "READ_ONLY", "READ_ONLY_NO_LOCK", "READ_ONLY_NO_LOCK_NO_ADMIN"}; > + > +static Sys_var_on_access_global<Sys_var_enum, > PRIV_SET_SYSTEM_GLOBAL_VAR_READ_ONLY> > Sys_readonly( > "read_only", > - "Make all non-temporary tables read-only, with the exception for " > - "replication (slave) threads and users with the 'READ ONLY ADMIN' " > - "privilege", > - GLOBAL_VAR(read_only), CMD_LINE(OPT_ARG), DEFAULT(FALSE), > - NO_MUTEX_GUARD, NOT_IN_BINLOG, > + "Do not allow changes to non-temporary tables. Options are:" > + "OFF changes allowed. " > + "READ_ONLY blocks changes for users without the 'READ ONLY ADMIN' " "Disallow changes for users without the READ ONLY ADMIN privilege" > + "privilege. " > + "READ_ONLY_NO_LOCK blocks in addition LOCK TABLES and " > + "SELECT IN SHARE MODE for not ADMIN users. " "Additionally disallows LOCK TABLES and SELECT IN SHARE MODE" > + "READ_ONLY_NO_LOCK_NO_ADMIN blocks in addition 'ADMIN' users. " "Disallows also for users with READ_ONLY ADMIN privilege" > + "Replication (slave) threads are not affected by this option", > + GLOBAL_VAR(read_only), CMD_LINE(OPT_ARG), > + read_only_mode_names, DEFAULT(0), NO_MUTEX_GUARD, NOT_IN_BINLOG, > ON_CHECK(check_read_only), ON_UPDATE(fix_read_only)); > > // Small lower limit to be able to test MRR Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1