Hi, Rucha! Looks quite good. A couple of small comments, see below. After you fix them, no need to ask for a new review, just push into bb-10.7-row_number On Oct 15, Rucha Deodhar wrote:
revision-id: 97458a7767b (mariadb-10.6.1-131-g97458a7767b) parent(s): a6cf8b34a83 author: Rucha Deodhar committer: Rucha Deodhar timestamp: 2021-10-15 01:54:51 +0530 message:
MDEV-26832: ROW_NUMBER in SIGNAL/RESIGNAL causes a syntax error
Analysis: Parser was missing ROW_NUMBER as syntax for SIGNAL and RESIGNAL. Fix: Fix parser and fix how m_row_number is copied like other attributes to avoid ROW_NUMBER from assuming default value.
diff --git a/mysql-test/main/get_diagnostics.test b/mysql-test/main/get_diagnostics.test index 83ea1dee343..186d1bfc438 100644 --- a/mysql-test/main/get_diagnostics.test +++ b/mysql-test/main/get_diagnostics.test @@ -1552,3 +1552,53 @@ GET DIAGNOSTICS CONDITION 2 @rnum = ROW_NUMBER, @msg = MESSAGE_TEXT, @err = MYSQ ... + +CREATE PROCEDURE resignal_syntax() +BEGIN + DECLARE CONTINUE HANDLER + FOR 1146 + BEGIN + RESIGNAL SET + MESSAGE_TEXT = '`temptab` does not exist', ROW_NUMBER= 105; + END; + SELECT `c` FROM `temptab`; +END| + +DELIMITER ;| + +--error ER_NO_SUCH_TABLE +CALL resignal_syntax();
You test that the above isn't a syntax error. But you also need to verify that row_number was, indeed, set. Like, do GET DIAGNOSTICS and print the ROW_NUMBER.
+ +DROP PROCEDURE resignal_syntax; diff --git a/sql/sql_error.cc b/sql/sql_error.cc index a2178831697..84065ddce15 100644 --- a/sql/sql_error.cc +++ b/sql/sql_error.cc @@ -190,6 +190,11 @@ static void copy_string(MEM_ROOT *mem_root, String* dst, const String* src) dst->length(0); }
+void copy_value(ulong *dst, const ulong *src) +{ + memcpy(dst, src, sizeof(*src)); +} + void Sql_condition::copy_opt_attributes(const Sql_condition *cond) { @@ -204,6 +209,7 @@ Sql_condition::copy_opt_attributes(const Sql_condition *cond) copy_string(m_mem_root, & m_table_name, & cond->m_table_name); copy_string(m_mem_root, & m_column_name, & cond->m_column_name); copy_string(m_mem_root, & m_cursor_name, & cond->m_cursor_name); + copy_value(& m_row_number, & cond->m_row_number);
That makes no sense. Other values were strings and had to be strdup-ed, but that's a number, you can simply do m_row_number= cond->m_row_number;
}
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org