August 2024 - developers - lists.mariadb.org

[Maria-developers] MariaDB // Temporal tables
by Robert Palm 02 Oct '24

02 Oct '24

2 2

Re: c46e7c5cb64: MDEV-12252 ROW data type for stored function return values
by Sergei Golubchik 05 Sep '24

05 Sep '24

Hi, Alexander, Looks good, thanks. Great commit comment too. Just a few questions and test suggestions - see below. Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org On Aug 23, Alexander Barkov wrote: > revision-id: c46e7c5cb64 (mariadb-11.4.1-5-gc46e7c5cb64) > parent(s): ae6684d79f4 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2024-02-20 13:49:54 +0400 > message: > > MDEV-12252 ROW data type for stored function return values > diff --git a/mysql-test/main/sp-anchor-row-type-table.test b/mysql-test/main/sp-anchor-row-type-table.test > index 7123f9160db..2fbcfbffc1b 100644 > --- a/mysql-test/main/sp-anchor-row-type-table.test > +++ b/mysql-test/main/sp-anchor-row-type-table.test > @@ -939,3 +939,70 @@ DROP TABLE t1; > --echo # > --echo # End of 10.4 tests > --echo # > + > +--echo # > +--echo # Start of 11.3 tests > +--echo # > + > +--echo # > +--echo # MDEV-12252 ROW data type for stored function return values > +--echo # > + > +--error ER_PARSE_ERROR > +CREATE FUNCTION f1() RETURNS ROW TYPE OF step1.step2.step3 RETURN ROW(1,2); > + > +CREATE FUNCTION f1() RETURNS ROW TYPE OF t1 RETURN ROW(1,2); you don't resolve t1 at CREATE FUNCTION time. why? is it how TYPE OF works in DECLARE too? > +SHOW CREATE FUNCTION f1; > +--error ER_NO_SUCH_TABLE > +SELECT f1(); > +DROP FUNCTION f1; > + > +CREATE FUNCTION f1() RETURNS ROW TYPE OF test.t1 RETURN ROW(1,2); > +SHOW CREATE FUNCTION f1; > +--error ER_NO_SUCH_TABLE > +SELECT f1(); > +DROP FUNCTION f1; > + > + > +CREATE TABLE t1 (a INT, b VARCHAR(32)); > +CREATE FUNCTION f1() RETURNS ROW TYPE OF test.t1 RETURN (SELECT * FROM t1); > +SHOW CREATE FUNCTION f1; > +DELIMITER $$; > +CREATE PROCEDURE p1() > +BEGIN > + DECLARE r ROW TYPE OF t1 DEFAULT f1(); > + SELECT r.a, r.b; > +END; > +$$ > +DELIMITER ;$$ > + > +# Testing with no rows > +CALL p1(); > +--error ER_OPERAND_COLUMNS > +SELECT f1(); > +SELECT f1()=ROW(1,'b1') AS c; > +SELECT f1()=ROW(NULL,NULL) AS c; > + > +# Testing with one row > +INSERT INTO t1 VALUES (1,'b1'); > +CALL p1(); > +--error ER_OPERAND_COLUMNS > +SELECT f1(); > +SELECT f1()=ROW(1,'b1') AS c; > +SELECT f1()=ROW(1,'') AS c; > +SELECT f1()=ROW(2,'b1') AS c; > +SELECT f1()=ROW(1,NULL) AS c; > +SELECT f1()=ROW(NULL,'b1') AS c; do one more SHOW CREATE FUNCTION here > + > +# Testing with two rows > +INSERT INTO t1 VALUES (2,'b2'); > +--error ER_SUBQUERY_NO_1_ROW > +CALL p1(); > +--error ER_OPERAND_COLUMNS > +SELECT f1(); > +--error ER_SUBQUERY_NO_1_ROW > +SELECT f1()=ROW(1,'b1') AS c; > + > +DROP PROCEDURE p1; > +DROP FUNCTION f1; > +DROP TABLE t1; > diff --git a/mysql-test/main/sp-anchor-type.test b/mysql-test/main/sp-anchor-type.test > index 0e24ef900d8..fed53fd011a 100644 > --- a/mysql-test/main/sp-anchor-type.test > +++ b/mysql-test/main/sp-anchor-type.test > @@ -767,3 +767,78 @@ BEGIN NOT ATOMIC > END; > $$ > DELIMITER ;$$ > + > + > +--echo # > +--echo # Start of 11.3 tests > +--echo # > + > +--echo # > +--echo # MDEV-12252 ROW data type for stored function return values > +--echo # > + > +--error ER_PARSE_ERROR > +CREATE FUNCTION f1() RETURNS TYPE OF a RETURN 1; > + > +CREATE FUNCTION f1() RETURNS TYPE OF t1.a RETURN (SELECT min(a) FROM t1); > +SHOW CREATE FUNCTION f1; > +--error ER_NO_SUCH_TABLE > +SELECT f1(); > +CREATE TABLE t1 (b INT); > +--error ER_BAD_FIELD_ERROR > +SELECT f1(); > +DROP TABLE t1; > +DROP FUNCTION f1; > + > +CREATE DATABASE db1; > +DELIMITER $$; > +CREATE FUNCTION db1.f1() RETURNS TYPE OF db1.t1.a > +BEGIN > + RETURN (SELECT min(a) FROM t1); > +END; > +$$ > +DELIMITER ;$$ > +SHOW CREATE FUNCTION db1.f1; > +--error ER_NO_SUCH_TABLE > +SELECT db1.f1(); > +CREATE TABLE db1.t1 (b TIME); > +--error ER_BAD_FIELD_ERROR > +SELECT db1.f1(); > +DROP TABLE db1.t1; > +CREATE TABLE db1.t1 (a TIME); > +SELECT db1.f1(); > +INSERT INTO db1.t1 VALUES ('10:20:30'); > +SELECT db1.f1(); > +DROP TABLE db1.t1; > +DROP FUNCTION db1.f1; > +DROP DATABASE db1; > + > +CREATE TABLE t1 (a TIME); > +CREATE FUNCTION f1() RETURNS TYPE OF test.t1.a RETURN (SELECT min(a) FROM t1); > +SHOW CREATE FUNCTION f1; > +SELECT f1(); > +DROP FUNCTION f1; > +DROP TABLE t1; > + > +CREATE TABLE t1 (a TIME); > +DELIMITER $$; > +CREATE FUNCTION f1() RETURNS TYPE OF t1.a > +BEGIN > + RETURN (SELECT min(a) FROM t1); > +END; > +$$ > +DELIMITER ;$$ > +SHOW CREATE FUNCTION f1; > +SELECT f1(); > +CREATE TABLE t2 AS SELECT f1(); > +SHOW CREATE TABLE t2; > +SELECT * FROM t2; > +DROP TABLE t2; > +INSERT INTO t1 VALUES ('10:20:30'); > +SELECT f1(); > +CREATE TABLE t2 AS SELECT f1(); > +SHOW CREATE TABLE t2; > +SELECT * FROM t2; > +DROP TABLE t2; please, also try here DROP TABLE t1; CREATE TABLE t1 (a INT); INSERT INTO t1 VALUES (10); CREATE TABLE t2 AS SELECT f1(); the point is to change TYPE OF t1.a after the function has been successfully called, and thus is already in the sp cache. > +DROP FUNCTION f1; > +DROP TABLE t1; > diff --git a/mysql-test/suite/compat/oracle/t/sp-package.test b/mysql-test/suite/compat/oracle/t/sp-package.test > index aefede41c8b..7c231d736c5 100644 > --- a/mysql-test/suite/compat/oracle/t/sp-package.test > +++ b/mysql-test/suite/compat/oracle/t/sp-package.test > @@ -3109,3 +3109,79 @@ DROP TABLE t1; > DROP FUNCTION f1_deterministic; > DROP FUNCTION f2_not_deterministic; > DROP PACKAGE pkg1; > + > +--echo # > +--echo # MDEV-12252 ROW data type for stored function return values > +--echo # > + > +--echo # > +--echo # Testing fixed ROW type with package routines > +--echo # > + > +DELIMITER $$; > +CREATE PACKAGE pkg > +AS > + FUNCTION f1() RETURN ROW(a INT, b VARCHAR(32)); > + PROCEDURE p1(r ROW(a INT, b VARCHAR(32))); > + PROCEDURE p2(); > +END; > +$$ > +CREATE PACKAGE BODY pkg > +AS > + FUNCTION f1() RETURN ROW(a INT, b VARCHAR(32)) AS > + BEGIN > + RETURN ROW(1,'b1'); > + END; > + PROCEDURE p1(r ROW(a INT, b VARCHAR(32))) AS > + BEGIN > + SELECT r.a, r.b; > + END; > + PROCEDURE p2() AS > + BEGIN > + CALL p1(f1()); > + END; > +END; > +$$ > +DELIMITER ;$$ > +CALL pkg.p1(pkg.f1()); > +CALL pkg.p2; > +DROP PACKAGE pkg; > + > + > +--echo # > +--echo # Testing table%ROWTYPE with package routines > +--echo # now you can also add package tests outside of oracle mode > + > +CREATE TABLE t1 (a INT, b VARCHAR(32)); > +INSERT INTO t1 VALUES (1,'b1'); > +DELIMITER /; > +CREATE PACKAGE pkg > +AS > + FUNCTION f1 RETURN t1%ROWTYPE; > + PROCEDURE p1(r t1%ROWTYPE); > + PROCEDURE p2; > +END; > +/ > +CREATE PACKAGE BODY pkg > +AS > + FUNCTION f1 RETURN t1%ROWTYPE AS > + r t1%ROWTYPE; > + BEGIN > + SELECT * INTO r FROM t1; > + RETURN r; > + END; > + PROCEDURE p1(r t1%ROWTYPE) AS > + BEGIN > + SELECT r.a || ' ' || r.b; > + END; > + PROCEDURE p2 AS > + BEGIN > + p1(f1()); > + END; > +END; > +/ > +DELIMITER ;/ > +CALL pkg.p1(pkg.f1()); > +CALL pkg.p2; > +DROP PACKAGE pkg; > +DROP TABLE t1; > diff --git a/sql/field.h b/sql/field.h > index 7f1c243a180..7af069a9735 100644 > --- a/sql/field.h > +++ b/sql/field.h > @@ -1373,6 +1374,20 @@ class Field: public Value_source > in str and restore it with set() if needed > */ > virtual void sql_type(String &str) const =0; > + void sql_type_for_sp_returns(String &str) const This is oddly specific. Strange that we don't need to print the field type with charset/collation anywhere else, but for sp returns we do. > + { > + sql_type(str); > + if (has_charset()) > + { > + str.append(STRING_WITH_LEN(" CHARSET ")); > + str.append(charset()->cs_name); > + if (Charset(charset()).can_have_collate_clause()) > + { > + str.append(STRING_WITH_LEN(" COLLATE ")); > + str.append(charset()->coll_name); > + } > + } > + } > virtual void sql_rpl_type(String *str) const { sql_type(*str); } > virtual uint size_of() const =0; // For new field > inline bool is_null(my_ptrdiff_t row_offset= 0) const > diff --git a/sql/field.cc b/sql/field.cc > index 35be045620d..23b050de221 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -2768,6 +2768,32 @@ bool Field_row::sp_prepare_and_store_item(THD *thd, Item **value) > } > > > +uint Field_row::cols() const > +{ > + // The table with ROW members must be already instantiated > + DBUG_ASSERT(m_table); > + return m_table->s->fields; > +} > + > + > +void Field_row::sql_type(String &res) const would be more logical to call this Field_row::sql_type_for_sp_returns() > +{ > + res.set_ascii(STRING_WITH_LEN("row(")); > + for (uint i= 0; i < m_table->s->fields; i++) > + { > + if (i > 0) > + res.append(','); > + Field *field= m_table->field[i]; > + res.append(field->field_name); > + res.append(' '); > + StringBuffer<64> col; > + field->sql_type_for_sp_returns(col); > + res.append(col.to_lex_cstring()); > + } > + res.append(')'); > +} > + > + > /**************************************************************************** > Functions for the Field_decimal class > This is an number stored as a pre-space (or pre-zero) string > diff --git a/sql/item.cc b/sql/item.cc > index 54ad511cb6b..0a125752979 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -3001,10 +3003,31 @@ Item_sp::init_result_field(THD *thd, uint max_length, uint maybe_null, > dummy_table->s->table_name= empty_clex_str; > dummy_table->maybe_null= maybe_null; > > + if (m_sp->m_return_field_def.is_column_type_ref() && > + m_sp->m_return_field_def.column_type_ref()-> > + resolve_type_ref(thd, &m_sp->m_return_field_def)) > + DBUG_RETURN(TRUE); Here you overwrite old m_return_field_def value which was a column_type_ref with its actual type, don't you? What will happen if you change the column's type after the sp was already executed once? > + > if (!(sp_result_field= m_sp->create_result_field(max_length, name, > dummy_table))) > DBUG_RETURN(TRUE); > > + /* > + In case of a ROW return type we need to create Item_field_row > + on top of Field_row, and remember it in sp_result_item_field_row. > + ROW members are later accessed using sp_result_item_field_row->addr(i), > + e.g. when copying the function return value to a local variable. > + For scalar return types no Item_field is needed around sp_result_field, > + as the value is fetched directly from sp_result_field, > + inside Item_func_sp::val_xxx() methods. Sorry, I don't understand that. Why cannot you use Field_row here? > + */ > + if (Field_row *field_row= dynamic_cast<Field_row*>(sp_result_field)) > + { > + if (!(sp_result_item_field_row= > + m_sp->m_return_field_def.make_item_field_row(thd, field_row))) > + DBUG_RETURN(true); > + } > + > if (sp_result_field->pack_length() > sizeof(result_buf)) > { > void *tmp; > diff --git a/sql/sp.cc b/sql/sp.cc > index ddaeeb8cb7a..7eab8304c31 100644 > --- a/sql/sp.cc > +++ b/sql/sp.cc > @@ -1091,18 +1136,19 @@ sp_returns_type(THD *thd, String &result, const sp_head *sp) > bzero((char*) &share, sizeof(share)); > table.in_use= thd; > table.s = &share; > - field= sp->create_result_field(0, 0, &table); > - field->sql_type(result); > + field= create_result_field(0, 0, &table); > > - if (field->has_charset()) > + if (m_return_field_def.is_row()) > { > - result.append(STRING_WITH_LEN(" CHARSET ")); > - result.append(field->charset()->cs_name); > - if (Charset(field->charset()).can_have_collate_clause()) > - { > - result.append(STRING_WITH_LEN(" COLLATE ")); > - result.append(field->charset()->coll_name); > - } > + Field_row *field_row= dynamic_cast<Field_row*>(field); > + if (!field_row->row_create_fields( > + thd, m_return_field_def.row_field_definitions())) > + field->sql_type(result); when is this used? you're creating a TABLE and Field's - only to print types and charsets. This might be tolerable, but only if it's not needed often > + } > + else > + { > + DBUG_ASSERT(m_return_field_def.type_handler()->is_scalar_type()); > + field->sql_type_for_sp_returns(result); > } > > delete field; > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index 5977e309ae4..f11e4f89466 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -18347,7 +18338,50 @@ sp_parameters: > ; > > sf_returned_type_clause: > - RETURNS_SYM sf_return_type > + RETURNS_SYM > + { > + LEX *lex= Lex; you still do this lex=Lex? :) it became obsolete, like, 15 years ago. If not more. > + lex->init_last_field(&lex->sphead->m_return_field_def, > + &empty_clex_str); > + } > + sf_return_type > + ; > + > +sf_return_type: > + field_type > + { > + if (unlikely(Lex->sf_return_fill_definition($1))) > + MYSQL_YYABORT; > + } > + | ROW_SYM row_type_body > + { > + if (Lex->sf_return_fill_definition_row($2)) > + MYSQL_YYABORT; > + } > + | ROW_SYM TYPE_SYM OF_SYM ident > + { > + if (Lex->sf_return_fill_definition_rowtype_of( > + Qualified_column_ident(&$4))) > + MYSQL_YYABORT; > + } > + | ROW_SYM TYPE_SYM OF_SYM ident '.' ident > + { > + if (Lex->sf_return_fill_definition_rowtype_of( > + Qualified_column_ident(&$4, &$6))) > + MYSQL_YYABORT; > + } > + | TYPE_SYM OF_SYM ident '.' ident > + { > + if (Lex->sf_return_fill_definition_type_of( > + Qualified_column_ident(&$3, &$5))) > + MYSQL_YYABORT; > + } > + | TYPE_SYM OF_SYM ident '.' ident '.' ident > + { > + if (Lex->sf_return_fill_definition_type_of( > + Qualified_column_ident(thd, &$3, &$5, &$7))) > + MYSQL_YYABORT; > + } > ; > > package_implementation_item_declaration:

2 1

Next LTS version in February 2025?
by Otto Kekäläinen 30 Aug '24

30 Aug '24

Hi! Traditionally there has been a new major MariaDB version in February of the year Debian has a new major version. What is the plan for February 2025? I was reading https://mariadb.org/about/#maintenance-policy and old mariadb.orb blog posts but none was explicit about this.

2 2

Re: fec3696fef6: MDEV-15751 CURRENT_TIMESTAMP should return a TIMESTAMP [WITH TIME ZONE?]
by Sergei Golubchik 29 Aug '24

29 Aug '24

Hi, Alexander, This has waited far too long :( > commit fec3696fef6 > Author: Alexander Barkov <bar(a)mariadb.com> > Date: Tue Feb 14 13:27:46 2023 +0400 > > diff --git a/sql/sql_class.h b/sql/sql_class.h > index f0bf695a1c1..7378225eaa2 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -3900,7 +3900,8 @@ class THD: public THD_count, /* this must be first */ > } > const Type_handler *type_handler_for_datetime() const; > bool timestamp_to_TIME(MYSQL_TIME *ltime, my_time_t ts, > - ulong sec_part, date_mode_t fuzzydate); > + ulong sec_part, date_mode_t fuzzydate, > + bool zero_timesteamp_means_zero_datetime); I don't understand it. NOW() cannot return a zero timestamp, how this feature can possibly need changes to zero timestamp treatment? > inline my_time_t query_start() { return start_time; } > inline ulong query_start_sec_part() > { used|= QUERY_START_SEC_PART_USED; return start_time_sec_part; } > diff --git a/sql/item_timefunc.h b/sql/item_timefunc.h > index a5f6d9307c6..1c463c8a332 100644 > --- a/sql/item_timefunc.h > +++ b/sql/item_timefunc.h > @@ -707,6 +707,49 @@ class Item_datetimefunc :public Item_func > }; > > > +class Item_timestampfunc: public Item_func > +{ > +protected: > + Datetime to_datetime(THD *thd) > + { > + return Timestamp_or_zero_datetime_native_null(thd, this).to_datetime(thd); how does it differ from Datetime(this) ? > + } > +public: > + Item_timestampfunc(THD *thd): Item_func(thd) {} > + Item_timestampfunc(THD *thd, Item *a): Item_func(thd, a) {} > + const Type_handler *type_handler() const override > + { return &type_handler_timestamp2; } > + bool get_date(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate) override > + { > + return null_value= to_datetime(thd).copy_to_mysql_time(ltime); > + } > + double val_real() override > + { > + Datetime dt= to_datetime(current_thd); > + null_value= !dt.is_valid_datetime(); > + return dt.to_double(); may be `return null_value ? 0 : dt.to_double()` ? why to call to_double if it isn't a valid datetime? (same everywhere below) > + } > + longlong val_int() override > + { > + Datetime dt= to_datetime(current_thd); > + null_value= !dt.is_valid_datetime(); > + return dt.to_longlong(); > + } > + my_decimal *val_decimal(my_decimal *to) override > + { > + Datetime dt= to_datetime(current_thd); > + null_value= !dt.is_valid_datetime(); > + return dt.to_decimal(to); > + } > + String *val_str(String *to) override > + { > + Datetime dt= to_datetime(current_thd); > + null_value= !dt.is_valid_datetime(); > + return dt.to_string(to, decimals); > + } > +}; > + > + > /* Abstract CURTIME function. Children should define what time zone is used */ > > class Item_func_curtime :public Item_timefunc > @@ -837,20 +880,48 @@ class Item_func_now :public Item_datetimefunc > }; > > > -class Item_func_now_local :public Item_func_now > +/* > + This is a replacement for Item_func_now_local, > + returning TIMESTAMP instead of DATETIME. why did you rename it? why didn't you change Item_func_now_utc? > +*/ > +class Item_func_current_timestamp: public Item_timestampfunc > { > public: > - Item_func_now_local(THD *thd, uint dec): Item_func_now(thd, dec) {} > + Item_func_current_timestamp(THD *thd, uint dec) > + :Item_timestampfunc(thd) > + { decimals= dec; } why not Item_timestampfunc(thd, dec) ? > LEX_CSTRING func_name_cstring() const override > { > static LEX_CSTRING name= {STRING_WITH_LEN("current_timestamp") }; > return name; > } > - int save_in_field(Field *field, bool no_conversions) override; > - void store_now_in_TIME(THD *thd, MYSQL_TIME *now_time) override; > + void print(String *str, enum_query_type query_type) override > + { > + str->append(func_name_cstring()); > + str->append('('); > + if (decimals) > + str->append_ulonglong(decimals); > + str->append(')'); > + } > + bool fix_length_and_dec(THD *thd) override > + { > + if (check_decimal_scale_or_error(TIME_SECOND_PART_DIGITS)) > + return true; > + fix_attributes_datetime(decimals); > + return false; > + } > + bool val_native(THD *thd, Native *to) override; > + bool check_vcol_func_processor(void *arg) override > + { > + /* > + NOW is safe for replication as slaves will run with same time as > + master > + */ what does this replication comment do in check_vcol_func_processor? (I know it was in the old code too, but "old code did it" never stopped your cleanups before :) > + return mark_unsupported_function(func_name(), "()", arg, VCOL_TIME_FUNC); > + } > enum Functype functype() const override { return NOW_FUNC; } > Item *get_copy(THD *thd) override > - { return get_item_copy<Item_func_now_local>(thd, this); } > + { return get_item_copy<Item_func_current_timestamp>(thd, this); } > }; > > > diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc > index 52b9ae7a682..3e224e3e6f7 100644 > --- a/sql/item_timefunc.cc > +++ b/sql/item_timefunc.cc > @@ -1642,32 +1633,10 @@ void Item_func_now::print(String *str, enum_query_type query_type) > } > > > -int Item_func_now_local::save_in_field(Field *field, bool no_conversions) > +bool Item_func_current_timestamp::val_native(THD *thd, Native *to) > { > - if (field->type() == MYSQL_TYPE_TIMESTAMP) > - { > - THD *thd= field->get_thd(); > - my_time_t ts= thd->query_start(); > - ulong sec_part= decimals ? thd->query_start_sec_part() : 0; > - sec_part-= my_time_fraction_remainder(sec_part, decimals); > - field->set_notnull(); > - field->store_timestamp(ts, sec_part); > - return 0; > - } > - else > - return Item_datetimefunc::save_in_field(field, no_conversions); > -} > - > - > -/** > - Converts current time in my_time_t to MYSQL_TIME representation for local > - time zone. Defines time zone (local) used for whole NOW function. > -*/ > -void Item_func_now_local::store_now_in_TIME(THD *thd, MYSQL_TIME *now_time) > -{ > - thd->variables.time_zone->gmt_sec_to_TIME(now_time, thd->query_start()); > - set_sec_part(thd->query_start_sec_part(), now_time, this); > - thd->used|= THD::TIME_ZONE_USED; > + Timestamp ts(Timeval(thd->query_start(), thd->query_start_sec_part())); > + return null_value= ts.trunc(decimals).to_native(to, decimals); can this possibly fail? > } > > > @@ -2732,7 +2693,6 @@ String *Item_func_tochar::val_str(String* str) > > bool Item_func_from_unixtime::fix_length_and_dec(THD *thd) > { > - thd->used|= THD::TIME_ZONE_USED; would be nice to have a test case for this. Like direct insert into a timestamp column no longer uses timezone - e.g. timestamp within dst switch time is inserted correctly. and vice versa, when from_unixtime is converted to a string, like `insert ... values(concat(from_unixtime(...)))` then TIME_ZONE_USED is still set and show binlog events (or mysqlbinlog) should show that the timezone was correctly binlogged, as before. > tz= thd->variables.time_zone; > Type_std_attributes::set( > Type_temporal_attributes_not_fixed_dec(MAX_DATETIME_WIDTH, Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1

MariaDB patches in Debian
by Otto Kekäläinen 27 Aug '24

27 Aug '24

Hi! Could perhaps Tuukka or Daniel review the patches currently carried in Debian if some of them would be applicable to be imported or re-implemented directly upstream so they can be dropped in Debian? https://udd.debian.org/patches.cgi?src=mariadb&version=1%3A11.4.3-1 Thanks

1 0

Re: [MariaDB commits] [PATCH 1/6] MDEV-34696: do_gco_wait() completes too early on InnoDB dict stats updates
by andrei.elkin＠pp.inet.fi 23 Aug '24

23 Aug '24

Howdy Kristian. > Before doing mark_start_commit(), check that there is no pending deadlock > kill. If there is a pending kill, we won't commit (we will abort, roll back, > and retry). Then we should not mark the commit as started, since that could > potentially make the following GCO start too early, before we completed the > commit after the retry. > > This condition could trigger in some corner cases, where InnoDB would take > temporarily table/row locks that are released again immediately, not held > until the transaction commits. This happens with dict_stats updates and > possibly auto-increment locks. > > Such locks can be passed to thd_rpl_deadlock_check() and cause a deadlock > kill to be scheduled in the background. But since the blocking locks are > held only temporarily, they can be released before the background kill > happens. This way, the kill can be delayed until after mark_start_commit() > has been called. Thus we need to check the synchronous indication > rgi->killed_for_retry, not just the asynchroneous thd->killed. I think I understood what's going on, also thanks to a verbose Jira ticket description. > > Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> > --- > sql/rpl_parallel.cc | 20 ++++++++++++++++---- > sql/rpl_rli.cc | 17 +++++++++++++++++ > 2 files changed, 33 insertions(+), 4 deletions(-) > > diff --git a/sql/rpl_parallel.cc b/sql/rpl_parallel.cc > index 1cfdf96ee3b..9c4222d7817 100644 > --- a/sql/rpl_parallel.cc > +++ b/sql/rpl_parallel.cc > @@ -1450,11 +1450,23 @@ handle_rpl_parallel_thread(void *arg) > after mark_start_commit(), we have to unmark, which has at least a > theoretical possibility of leaving a window where it looks like all > transactions in a GCO have started committing, while in fact one > - will need to rollback and retry. This is not supposed to be possible > - (since there is a deadlock, at least one transaction should be > - blocked from reaching commit), but this seems a fragile ensurance, > - and there were historically a number of subtle bugs in this area. > + will need to rollback and retry. > + > + Normally this will not happen, since the kill is there to resolve a > + deadlock that is preventing at least one transaction from proceeding. > + One case it can happen is with InnoDB dict stats update, which can > + temporarily cause transactions to block each other, but locks are > + released immediately, they don't linger until commit. There could be > + other similar cases, there were historically a number of subtle bugs > + in this area. > + > + But once we start the commit, we can expect that no new lock > + conflicts will be introduced. So by handling any lingering deadlock > + kill at this point just before mark_start_commit(), we should be > + robust even towards spurious deadlock kills. > */ > + if (rgi->killed_for_retry != rpl_group_info::RETRY_KILL_NONE) > + wait_for_pending_deadlock_kill(thd, rgi); Assuming my understanding, please correct me if anything, I left a question on this block in https://github.com/MariaDB/server/commit/df0c36a354ffde2766ebed2615642c74b7… quoted further. ... this guard would not let a victim of an optimistic conflict proceed until it clears out itself it is such a victim. For instance in the binlog sequence like T_1, T_2, D_3 where T_2 is the victim, D_3 is a DDL, the victim T_2 won't anymore release the next gco member D_3 into its execution. But what if at this point T_2 is only going to be marked in rgi->killed_for_retry by T_1? That is it is apparently able, with this patch as well, having ...NONE killed status go through. T_1 would reach this very point later and when that is done before T_2 finds itself killed, T_1 would release D_3, and afterward T_2 would finally see itself KILLed into retry. Cheers, Andrei

2 4

Open PRs pending second approver (has already one)
by Otto Kekäläinen 23 Aug '24

23 Aug '24

Hi! There are couple PRs that have 1 approval, and are pending the second one. Would somebody have time to review them? In order of importance: https://github.com/MariaDB/server/pull/3401 "MDEV-34680 Asynchronous and Buffered Logging in MySQL Audit Plugin" https://github.com/MariaDB/server/pull/3406 (related but zero approvals: https://github.com/MariaDB/server/pull/3324 "MDEV-12182 Add client tcp port to audit plugin logs") https://github.com/MariaDB/server/pull/3208 "Refine OpenSSL test names and allow more complete OpenSSL version coverage" https://github.com/MariaDB/server/pull/3225 "MDEV-29955: add ${PCRE_LIBRARY_PATH} to link directories" Thanks, Otto

1 0

Re: 186aa192d1f: MDEV-34704 Quick mode produces the bug for mariadb client
by Sergei Golubchik 22 Aug '24

22 Aug '24

Hi, Sanja, ok to push, but see one comment below On Aug 22, Oleksandr Byelkin wrote: > revision-id: 186aa192d1f (mariadb-11.5.1-34-g186aa192d1f) > parent(s): 9e1923ca238 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2024-08-05 16:16:09 +0200 > message: > > MDEV-34704 Quick mode produces the bug for mariadb client > > --quick-max-column-width parameter added to limit field > width in --quick mode. > > diff --git a/mysql-test/main/client.test b/mysql-test/main/client.test > new file mode 100644 > index 00000000000..59e687ff8b9 > --- /dev/null > +++ b/mysql-test/main/client.test > @@ -0,0 +1,33 @@ > +--echo # > +--echo # MDEV-34704: Quick mode produces the bug for mariadb client > +--echo # > + > + > +create table t1 (aaaaaaaaa char (5), aaaaa char (10), a char (127), b char(1)); > +insert into t1 values ("X", "X", "X", "X"); > + to make the result file more readable, I'd suggest to add here, like --echo mysql --table --quick ... --echo mysql --table --quick --quick-max-column-width=0 ... etc before every corresponding --exec > +--exec echo "select * from test.t1;" | $MYSQL --table --quick 2>&1 > + > +--exec echo "select * from test.t1;" | $MYSQL --table --quick --quick-max-column-width=0 2>&1 > + > +--exec echo "select * from test.t1;" | $MYSQL --table --quick --quick-max-column-width=10 2>&1 > + > +--exec echo "select * from test.t1;" | $MYSQL --table --quick --quick-max-column-width=20 2>&1 > + > +insert into t1 values ("01234", "0123456789", "01234567890123456789", "1"); > + > +--exec echo "select * from test.t1;" | $MYSQL --table --quick 2>&1 > + > + > +--exec echo "select * from test.t1;" | $MYSQL --table --quick --quick-max-column-width=0 2>&1 > + > +--exec echo "select * from test.t1;" | $MYSQL --table --quick --quick-max-column-width=10 2>&1 > + > +--exec echo "select * from test.t1;" | $MYSQL --table --quick --quick-max-column-width=20 2>&1 > + > +drop table t1; > + > +--echo # > +--echo # End of 10.7 tests > +--echo # As Marko said, use here only one line --echo # End of 10.7 tests otherwise merge mixes it up with --echo # --echo # MDEV-34704: Quick mode produces the bug for mariadb client --echo # Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

Suggestion: use 'main' as the development branch instead of an ever changing 11.x
by Otto Kekäläinen 22 Aug '24

22 Aug '24

Hi! Has the core developers had discussions about using branch name 'main' for development instead of switching to a new branch every 3 months? If all new features and other additions would always target the branch 'main' it would lower the barrier of entry for new contributors, and decrease the number of unnecessary rebases done by contributors. If the permanent development target was 'main', you would need to create new 11.x branches only at "freeze" when a new major version branch is cut for release. MariaDB is currently the only open source project I know that does *not* have a main/master/trunk branch and instead does these constant new branch names. I imagine this is not only annoying for contributors, but also the core developers who waste time asking contributors to rebase PRs every 3 months (see https://github.com/MariaDB/server/pull/2556 and https://github.com/MariaDB/server/pull/2671 as examples). - Otto

7 11

Re: [MariaDB commits] [PATCH 3/6] Fix sporadic test failure in rpl.rpl_create_drop_event
by Kristian Nielsen 22 Aug '24

22 Aug '24

Andrei Elkin <andrei.elkin(a)mariadb.com> writes: > Kristian Nielsen via developers <developers(a)lists.mariadb.org> writes: >> The already-started instance of the event happily continues to run after >> being dropped. I wonder if that's a bug, or if the documentation is >> wrong? > > To this question, I vaguely remember discussions about this matter which > must've concuded in that it's not a bug when the scheduler races ahead > of DROP EVENT to launch a piece of job, like INSERT in this case. Aha, I see. Following up on this, I found this comment in the code Stops the scheduler (again). Waits for acknowledgement from the scheduler that it has stopped - synchronous stopping. Already running events will not be stopped. If the user needs them stopped manual intervention is needed. So then the behaviour seen in the test case is expected, and just fixing the test to handle the possible race should be correct. I also see in the code that the thread running the events starts by loading the event definition from the system tables (mysql.event ...), db_repository->load_named_event(). So there should be no memory issues even if the event is dropped while an instance of it is running. Thanks! - Kristian.

1 0