MDEV-4824 userstats - wrong user statistics

(and valgrind warnings)

* move thd userstat initialization to the same function
  that was adding thd userstat to global counters.
* initialize thd->start_bytes_received in THD::init
  (when thd->userstat_running is set)

mysql-test/r/userstat-badlogin-4824.result

@@ -0,0 +1,16 @@
+create user foo@localhost identified by 'foo';
+flush user_statistics;
+set global userstat=1;
+select 1;
+1
+1
+select user, bytes_received from information_schema.user_statistics where user = 'foo';
+user	bytes_received
+foo	18
+connect(localhost,foo,bar,test,MASTER_PORT,MASTER_SOCKET);
+ERROR 28000: Access denied for user 'foo'@'localhost' (using password: YES)
+select user, bytes_received from information_schema.user_statistics where user = 'foo';
+user	bytes_received
+foo	18
+drop user foo@localhost;
+set global userstat=0;

mysql-test/t/userstat-badlogin-4824.test

@@ -0,0 +1,33 @@
+#
+# MDEV-4824 userstats - wrong user statistics
+#
+--source include/not_embedded.inc
+
+create user foo@localhost identified by 'foo';
+flush user_statistics;
+set global userstat=1;
+
+connect(foo, localhost, foo, foo);
+select 1;
+disconnect foo;
+connection default;
+
+# wait for user_statistics changes to become visible
+let $wait_condition= select count(*) = 1 from information_schema.processlist;
+--source include/wait_condition.inc
+
+# 41 is for ps-procotol
+--replace_result 41 18
+select user, bytes_received from information_schema.user_statistics where user = 'foo';
+
+--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
+--error ER_ACCESS_DENIED_ERROR
+connect(foo, localhost, foo, bar);
+
+connection default;
+
+--replace_result 41 18
+select user, bytes_received from information_schema.user_statistics where user = 'foo';
+
+drop user foo@localhost;
+set global userstat=0;

sql/event_data_objects.cc

@@ -1337,7 +1337,7 @@ Event_job_data::execute(THD *thd, bool drop)
 
   DBUG_ENTER("Event_job_data::execute");
 
-  mysql_reset_thd_for_next_command(thd, 0);
+  mysql_reset_thd_for_next_command(thd);
 
   /*
     MySQL parser currently assumes that current database is either

sql/log_event.cc

@@ -5353,7 +5353,7 @@ int Load_log_event::do_apply_event(NET* net, Relay_log_info const *rli,
   */
   lex_start(thd);
   thd->lex->local_file= local_fname;
-  mysql_reset_thd_for_next_command(thd, 0);
+  mysql_reset_thd_for_next_command(thd);
 
   if (!use_rli_only_for_errors)
   {
@@ -7353,7 +7353,7 @@ int Append_block_log_event::do_apply_event(Relay_log_info const *rli)
       as the present method does not call mysql_parse().
     */
     lex_start(thd);
-    mysql_reset_thd_for_next_command(thd, 0);
+    mysql_reset_thd_for_next_command(thd);
     /* old copy may exist already */
     mysql_file_delete(key_file_log_event_data, fname, MYF(0));
     if ((fd= mysql_file_create(key_file_log_event_data,
@@ -8308,7 +8308,7 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli)
       we need to do any changes to that value after this function.
     */
     lex_start(thd);
-    mysql_reset_thd_for_next_command(thd, 0);
+    mysql_reset_thd_for_next_command(thd);
     /*
       The current statement is just about to begin and 
       has not yet modified anything. Note, all.modified is reset

sql/log_event_old.cc

@@ -87,7 +87,7 @@ Old_rows_log_event::do_apply_event(Old_rows_log_event *ev, const Relay_log_info
       we need to do any changes to that value after this function.
     */
     lex_start(ev_thd);
-    mysql_reset_thd_for_next_command(ev_thd, 0);
+    mysql_reset_thd_for_next_command(ev_thd);
 
     /*
       This is a row injection, so we flag the "statement" as

sql/sql_class.cc

@@ -1260,6 +1260,7 @@ void THD::init(void)
   reset_current_stmt_binlog_format_row();
   bzero((char *) &status_var, sizeof(status_var));
   bzero((char *) &org_status_var, sizeof(org_status_var));
+  start_bytes_received= 0;
 
   if (variables.sql_log_bin)
     variables.option_bits|= OPTION_BIN_LOG;

sql/sql_class.h

@@ -1556,7 +1556,7 @@ public:
   /* Slave applier execution context */
   Relay_log_info* rli_slave;
 
-  void reset_for_next_command(bool calculate_userstat);
+  void reset_for_next_command();
   /*
     Constant for THD::where initialization in the beginning of every query.
 

sql/sql_parse.cc

@@ -717,7 +717,6 @@ bool do_command(THD *thd)
 
   net_new_transaction(net);
 
-
   /* Save for user statistics */
   thd->start_bytes_received= thd->status_var.bytes_received;
 
@@ -926,6 +925,14 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
   if (!(server_command_flags[command] & CF_SKIP_QUESTIONS))
     statistic_increment(thd->status_var.questions, &LOCK_status);
 
+  /* Copy data for user stats */
+  if ((thd->userstat_running= opt_userstat_running))
+  {
+    thd->start_cpu_time= my_getcputime();
+    memcpy(&thd->org_status_var, &thd->status_var, sizeof(thd->status_var));
+    thd->select_commands= thd->update_commands= thd->other_commands= 0;
+  }
+
   /**
     Clear the set of flags that are expected to be cleared at the
     beginning of each command.
@@ -1176,7 +1183,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
       break;
     }
     packet= arg_end + 1;
-    mysql_reset_thd_for_next_command(thd, opt_userstat_running);
+    mysql_reset_thd_for_next_command(thd);
     lex_start(thd);
     /* Must be before we init the table list. */
     if (lower_case_table_names)
@@ -5446,15 +5453,15 @@ bool my_yyoverflow(short **yyss, YYSTYPE **yyvs, ulong *yystacksize)
 
   @todo Call it after we use THD for queries, not before.
 */
-void mysql_reset_thd_for_next_command(THD *thd, my_bool calculate_userstat)
+void mysql_reset_thd_for_next_command(THD *thd)
 {
-  thd->reset_for_next_command(calculate_userstat);
+  thd->reset_for_next_command();
 }
 
-void THD::reset_for_next_command(bool calculate_userstat)
+void THD::reset_for_next_command()
 {
   THD *thd= this;
-  DBUG_ENTER("mysql_reset_thd_for_next_command");
+  DBUG_ENTER("THD::reset_for_next_command");
   DBUG_ASSERT(!thd->spcont); /* not for substatements of routines */
   DBUG_ASSERT(! thd->in_sub_stmt);
   DBUG_ASSERT(thd->transaction.on);
@@ -5500,14 +5507,6 @@ void THD::reset_for_next_command(bool calculate_userstat)
   thd->sent_row_count= thd->examined_row_count= 0;
   thd->accessed_rows_and_keys= 0;
 
-  /* Copy data for user stats */
-  if ((thd->userstat_running= calculate_userstat))
-  {
-    thd->start_cpu_time= my_getcputime();
-    memcpy(&thd->org_status_var, &thd->status_var, sizeof(thd->status_var));
-    thd->select_commands= thd->update_commands= thd->other_commands= 0;
-  }
-
   thd->query_plan_flags= QPLAN_INIT;
   thd->query_plan_fsort_passes= 0;
 
@@ -5716,7 +5715,7 @@ void mysql_parse(THD *thd, char *rawbuf, uint length,
     FIXME: cleanup the dependencies in the code to simplify this.
   */
   lex_start(thd);
-  mysql_reset_thd_for_next_command(thd, opt_userstat_running);
+  mysql_reset_thd_for_next_command(thd);
 
   if (query_cache_send_result_to_client(thd, rawbuf, length) <= 0)
   {
@@ -5819,7 +5818,7 @@ bool mysql_test_parse_for_slave(THD *thd, char *rawbuf, uint length)
   if (!(error= parser_state.init(thd, rawbuf, length)))
   {
     lex_start(thd);
-    mysql_reset_thd_for_next_command(thd, opt_userstat_running);
+    mysql_reset_thd_for_next_command(thd);
 
     if (!parse_sql(thd, & parser_state, NULL) &&
         all_tables_not_ok(thd, lex->select_lex.table_list.first))

sql/sql_parse.h

@@ -83,7 +83,7 @@ bool alloc_query(THD *thd, const char *packet, uint packet_length);
 void mysql_init_select(LEX *lex);
 void mysql_parse(THD *thd, char *rawbuf, uint length,
                  Parser_state *parser_state);
-void mysql_reset_thd_for_next_command(THD *thd, my_bool calculate_userstat);
+void mysql_reset_thd_for_next_command(THD *thd);
 bool mysql_new_select(LEX *lex, bool move_down);
 void create_select_for_variable(const char *var_name);
 void create_table_set_open_action_and_adjust_tables(LEX *lex);

sql/sql_prepare.cc

@@ -2269,7 +2269,7 @@ void mysqld_stmt_prepare(THD *thd, const char *packet, uint packet_length)
   DBUG_PRINT("prep_query", ("%s", packet));
 
   /* First of all clear possible warnings from the previous command */
-  mysql_reset_thd_for_next_command(thd, opt_userstat_running);
+  mysql_reset_thd_for_next_command(thd);
 
   if (! (stmt= new Prepared_statement(thd)))
     goto end;           /* out of memory: error is set in Sql_alloc */
@@ -2659,7 +2659,7 @@ void mysqld_stmt_execute(THD *thd, char *packet_arg, uint packet_length)
   packet+= 9;                               /* stmt_id + 5 bytes of flags */
 
   /* First of all clear possible warnings from the previous command */
-  mysql_reset_thd_for_next_command(thd, opt_userstat_running);
+  mysql_reset_thd_for_next_command(thd);
 
   if (!(stmt= find_prepared_statement(thd, stmt_id)))
   {
@@ -2758,7 +2758,7 @@ void mysqld_stmt_fetch(THD *thd, char *packet, uint packet_length)
   DBUG_ENTER("mysqld_stmt_fetch");
 
   /* First of all clear possible warnings from the previous command */
-  mysql_reset_thd_for_next_command(thd, opt_userstat_running);
+  mysql_reset_thd_for_next_command(thd);
 
   status_var_increment(thd->status_var.com_stmt_fetch);
   if (!(stmt= find_prepared_statement(thd, stmt_id)))
@@ -2818,7 +2818,7 @@ void mysqld_stmt_reset(THD *thd, char *packet)
   DBUG_ENTER("mysqld_stmt_reset");
 
   /* First of all clear possible warnings from the previous command */
-  mysql_reset_thd_for_next_command(thd, opt_userstat_running);
+  mysql_reset_thd_for_next_command(thd);
 
   status_var_increment(thd->status_var.com_stmt_reset);
   if (!(stmt= find_prepared_statement(thd, stmt_id)))