Re: [Maria-developers] 4156071ebe2: MDEV-26606: ERROR_INDEX property value isn't passed from inside a stored procedure
![](https://secure.gravatar.com/avatar/39b623a1559cf9c69ac3d9d4fb44e7fe.jpg?s=120&d=mm&r=g)
Hi, Rucha! On Sep 28, Rucha Deodhar wrote:
revision-id: 4156071ebe2 (mariadb-10.6.1-91-g4156071ebe2) parent(s): 8dd4794c4e1 author: Rucha Deodhar committer: Rucha Deodhar timestamp: 2021-09-26 19:58:33 +0530 message:
MDEV-26606: ERROR_INDEX property value isn't passed from inside a stored procedure
Analysis: m_current_row_for_warning is reset to 1 during cleanup phase of stored procedure. When we perform a copy because some statement of procedure created warning, this reset value os passed to push _warning(). Hence the output is always 1. Fix: Add a parameter in relevant functions to pass correct value of error index and don't use m_current_row_for_warning directly.
first thought about test cases, ERROR_INDEX should show the same row number as in a warning, so let's check whether warning is correct or not too. In this case we have CREATE OR REPLACE TABLE t1 (pk TINYINT PRIMARY KEY); CREATE OR REPLACE PROCEDURE sp(a INT) INSERT INTO t1 VALUES (2),(a); INSERT INTO t1 VALUES(1); --error ER_WARN_DATA_OUT_OF_RANGE CALL sp(100000); prints ERROR 22003: Out of range value for column 'a' at row 2 which is fine. Would be good to have it in the test. Otherwise looks good. One question below.
diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 31f5faa72ed..4d7093763ae 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1149,7 +1149,8 @@ Sql_condition* THD::raise_condition(uint sql_errno, if (likely(!(is_fatal_error && (sql_errno == EE_OUTOFMEMORY || sql_errno == ER_OUTOFMEMORY)))) { - cond= da->push_warning(this, sql_errno, sqlstate, level, ucid, msg); + cond= da->push_warning(this, sql_errno, sqlstate, level, ucid, msg, + da->current_row_for_warning()); } DBUG_RETURN(cond); } diff --git a/sql/sql_error.cc b/sql/sql_error.cc index 7b6e6a83d2f..ca9da3dbf89 100644 --- a/sql/sql_error.cc +++ b/sql/sql_error.cc @@ -663,7 +663,8 @@ void Warning_info::reserve_space(THD *thd, uint count)
Sql_condition *Warning_info::push_warning(THD *thd, const Sql_condition_identity *value, - const char *msg) + const char *msg, + ulong current_error_index) { Sql_condition *cond= NULL;
@@ -673,7 +674,7 @@ Sql_condition *Warning_info::push_warning(THD *thd, m_warn_list.elements() < thd->variables.max_error_count) { cond= new (& m_warn_root) Sql_condition(& m_warn_root, *value, msg, - m_current_row_for_warning); + current_error_index); if (cond) m_warn_list.push_back(cond); } @@ -689,7 +690,8 @@ Sql_condition *Warning_info::push_warning(THD *thd, const Sql_condition *sql_condition) { Sql_condition *new_condition= push_warning(thd, sql_condition, - sql_condition->get_message_text()); + sql_condition->get_message_text(), + sql_condition->m_error_index);
if (new_condition) new_condition->copy_opt_attributes(sql_condition); diff --git a/sql/sql_error.h b/sql/sql_error.h index b6ed7f5061a..a3e00e8419d 100644 --- a/sql/sql_error.h +++ b/sql/sql_error.h @@ -747,7 +747,8 @@ class Warning_info */ Sql_condition *push_warning(THD *thd, const Sql_condition_identity *identity, - const char* msg); + const char* msg, + ulong current_error_index);
/** Add a new SQL-condition to the current list and increment the respective @@ -1178,10 +1179,12 @@ class Diagnostics_area: public Sql_state_errno, const char* sqlstate, Sql_condition::enum_warning_level level, const Sql_user_condition_identity &ucid, - const char* msg) + const char* msg, + ulong current_error_index) { Sql_condition_identity tmp(sql_errno_arg, sqlstate, level, ucid); - return get_warning_info()->push_warning(thd, &tmp, msg); + return get_warning_info()->push_warning(thd, &tmp, msg, + current_error_index); }
Sql_condition *push_warning(THD *thd, @@ -1191,7 +1194,7 @@ class Diagnostics_area: public Sql_state_errno, const char* msg) { return push_warning(thd, sqlerrno, sqlstate, level, - Sql_user_condition_identity(), msg); + Sql_user_condition_identity(), msg, 0);
what is this push_warning used for? does it never need a proper current_error_index?
} void mark_sql_conditions_for_removal() { get_warning_info()->mark_sql_conditions_for_removal(); }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
participants (1)
-
Sergei Golubchik