maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Nirbhay!

On Nov 28, nirbhay@xxxxxxxxxxx wrote:
> revision-id: 7a5559421f4f3dda44d04bf946e79661d09b4dc2
> parent(s): 7b55b67de5fdfe91283357fe6c7ccc3f9e355925
> committer: Nirbhay Choubey
> branch nick: b4412
> timestamp: 2014-11-28 20:53:02 -0500
> message:
> 
> MDEV-4412 : SLOW QUERY LOG - add affected rows (UPDATE / DELETE) in slow query log
> 
> Added Rows_affected to slow query log & mysql.slow_log table.

Just a couple of comments:

> diff --git a/scripts/mysql_system_tables_fix.sql b/scripts/mysql_system_tables_fix.sql
> index b9bb59c..4973dc9 100644
> --- a/scripts/mysql_system_tables_fix.sql
> +++ b/scripts/mysql_system_tables_fix.sql
> @@ -262,7 +262,8 @@ ALTER TABLE slow_log
>    MODIFY insert_id INTEGER NOT NULL,
>    MODIFY server_id INTEGER UNSIGNED NOT NULL,
>    MODIFY sql_text MEDIUMTEXT NOT NULL,
> -  MODIFY thread_id BIGINT(21) UNSIGNED NOT NULL;
> +  MODIFY thread_id BIGINT(21) UNSIGNED NOT NULL,
> +  MODIFY rows_affected INTEGER NOT NULL;

Eh? That makes no sense. This file is for mysql_upgrade.
Your ALTER TABLE would help if there was some MariaDB version
where rows_affected column would've been nullable and you
wanted to change it to NOT NULL. But there was no such MariaDB version.
There was MariaDB version without rows_affected column at all.
So, instead of this MODIFY rows_affected, you need to put
another ALTER TABLE statement that adds rows_affected INTEGER NOT NULL column.

>  SET GLOBAL slow_query_log = @old_log_state;
>  
>  ALTER TABLE plugin
> diff --git a/sql/log.cc b/sql/log.cc
> index ff04105..24e5283 100644
> --- a/sql/log.cc
> +++ b/sql/log.cc
> @@ -2918,12 +2924,16 @@ bool MYSQL_QUERY_LOG::write(THD *thd, time_t current_time,
>      sprintf(lock_time_buff,  "%.6f", ulonglong2double(lock_utime)/1000000.0);
>      if (my_b_printf(&log_file,
>                      "# Thread_id: %lu  Schema: %s  QC_hit: %s\n" \
> -                    "# Query_time: %s  Lock_time: %s  Rows_sent: %lu  Rows_examined: %lu\n",
> +                    "# Query_time: %s  Lock_time: %s  Rows_sent: %lu  Rows_examined: %lu\n" \
> +                    "# Rows_affected: %lu\n",

I'd rather have Rows_affected on the same line with Rows_sent and Rows_examined

>                      (ulong) thd->thread_id, (thd->db ? thd->db : ""),
>                      ((thd->query_plan_flags & QPLAN_QC) ? "Yes" : "No"),
>                      query_time_buff, lock_time_buff,
>                      (ulong) thd->get_sent_row_count(),
> -                    (ulong) thd->get_examined_row_count()) == (size_t) -1)
> +                    (ulong) thd->get_examined_row_count(),
> +                    thd->get_stmt_da()->is_ok() ?
> +                    (ulong) thd->get_stmt_da()->affected_rows() :
> +                    0) == (size_t) -1)
>        tmp_errno= errno;
>       if ((thd->variables.log_slow_verbosity & LOG_SLOW_VERBOSITY_QUERY_PLAN) &&
>           (thd->query_plan_flags &

Regards,
Sergei