Re: 64ac6ed3e35: MDEV-31742 incorrect examined rows in case of stored function usage
Hi, Oleksandr, On Jul 21, Oleksandr Byelkin wrote:
revision-id: 64ac6ed3e35 (mariadb-10.4.30-31-g64ac6ed3e35) parent(s): 55a27bac80c author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2023-07-19 16:14:39 +0200 message:
MDEV-31742 incorrect examined rows in case of stored function usage
The counter is global so we do not need add backup to it if we do not zero it after taking the backup.
diff --git a/sql/sql_class.cc b/sql/sql_class.cc index c9e86b6142c..7911c78b6e0 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -5568,7 +5568,6 @@ void THD::restore_sub_statement_state(Sub_statement_state *backup) The following is added to the old values as we are interested in the total complexity of the query */ - inc_examined_row_count(backup->examined_row_count);
may be backup->examined_row_count should be simply removed? Sub_statement_state::examined_row_count, I mean. Not saved, not restored? There's also void THD::add_slow_query_state(Sub_statement_state *backup) { affected_rows+= backup->affected_rows; bytes_sent_old= backup->bytes_sent_old; m_examined_row_count+= backup->examined_row_count; it's not even using inc_examined_row_count(). Also examined_row_count is mentioned in comments in sql_class.cc, if you'll be removing it from Sub_statement_state, don't forget to fix comments to match.
cuted_fields+= backup->cuted_fields; DBUG_VOID_RETURN; }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
I also thought about removing it, but then decided to let the author to fix it, here is an urgent fix because as the previous bug showed, our client hit this problem. There is also "cuted_fields+= backup->cuted_fields;" which is very suspicious. On Fri, Jul 21, 2023 at 11:41 AM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Oleksandr,
On Jul 21, Oleksandr Byelkin wrote:
revision-id: 64ac6ed3e35 (mariadb-10.4.30-31-g64ac6ed3e35) parent(s): 55a27bac80c author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2023-07-19 16:14:39 +0200 message:
MDEV-31742 incorrect examined rows in case of stored function usage
The counter is global so we do not need add backup to it if we do not zero it after taking the backup.
diff --git a/sql/sql_class.cc b/sql/sql_class.cc index c9e86b6142c..7911c78b6e0 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -5568,7 +5568,6 @@ void THD::restore_sub_statement_state(Sub_statement_state *backup) The following is added to the old values as we are interested in the total complexity of the query */ - inc_examined_row_count(backup->examined_row_count);
may be backup->examined_row_count should be simply removed? Sub_statement_state::examined_row_count, I mean. Not saved, not restored?
There's also
void THD::add_slow_query_state(Sub_statement_state *backup) { affected_rows+= backup->affected_rows; bytes_sent_old= backup->bytes_sent_old; m_examined_row_count+= backup->examined_row_count;
it's not even using inc_examined_row_count().
Also examined_row_count is mentioned in comments in sql_class.cc, if you'll be removing it from Sub_statement_state, don't forget to fix comments to match.
cuted_fields+= backup->cuted_fields; DBUG_VOID_RETURN; }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi, Sanja, Hmm, what author do you have in mind, Timour? I suggest to remove this now and not leave dead code around. On Jul 21, Oleksandr Byelkin wrote:
I also thought about removing it, but then decided to let the author to fix it, here is an urgent fix because as the previous bug showed, our client hit this problem.
There is also "cuted_fields+= backup->cuted_fields;" which is very suspicious.
On Fri, Jul 21, 2023 at 11:41 AM Sergei Golubchik <serg@mariadb.org> wrote:
On Jul 21, Oleksandr Byelkin wrote:
revision-id: 64ac6ed3e35 (mariadb-10.4.30-31-g64ac6ed3e35) parent(s): 55a27bac80c author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2023-07-19 16:14:39 +0200 message:
MDEV-31742 incorrect examined rows in case of stored function usage
The counter is global so we do not need add backup to it if we do not zero it after taking the backup.
may be backup->examined_row_count should be simply removed? Sub_statement_state::examined_row_count, I mean. Not saved, not restored?
There's also
void THD::add_slow_query_state(Sub_statement_state *backup) { affected_rows+= backup->affected_rows; bytes_sent_old= backup->bytes_sent_old; m_examined_row_count+= backup->examined_row_count;
it's not even using inc_examined_row_count().
Also examined_row_count is mentioned in comments in sql_class.cc, if you'll be removing it from Sub_statement_state, don't forget to fix comments to match.
cuted_fields+= backup->cuted_fields; DBUG_VOID_RETURN; }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
participants (2)
-
Oleksandr Byelkin -
Sergei Golubchik