maria-developers team mailing list archive

[Nirbhay Choubey <nirbhay@xxxxxxxxxxx>]

Hi Serg,

On Sat, Nov 29, 2014 at 1:54 PM, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> 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.
>

Right, I have fixed this.


>
> >  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
>

Based on how mysqldumpslow parses the slow log file, IMO adding a new
attribute to the
same line may render the existing line non-parsable to older versions. I
also had to make
a tiny modification to the script for it to consider "QC_hit:".

Best,
Nirbhay


> >                      (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
>