Skip to content

Commit

Permalink
Fix row count for audit plugin
Browse files Browse the repository at this point in the history
Summary:
The audit plugin implementation MySQL is reusing a variable that does not always reflect rows sent to client. This means that that in many cases, it is not counting the actual rows sent to the client. There is a more accurate count on the THD object that should be used instead.

I am ignoring the documentation so that it just returns the number of rows returned the client: (https://dev.mysql.com/doc/refman/5.5/en/writing-audit-plugins.html)

I am also adding new fields that will be passed into the audit plugin. This includes, number of affected rows, the connection certificate, and query attributes. Query attributes and connection certificates are currently not implemented, but will come shortly.

If the distinction between commands that return no results, and commands that return empty results is important, then look at affected rows. For commands that return no results, affected rows is non-negative. For SELECTs that return empty results, affected rows is -1.

Test Plan:
Tested manually in gdb with SELECT/INSERT/UPDATE/DELETE queries:

  create table t (i int);
  select * from t;
  result rows: 0
  affect rows: -1

  insert into t values (1), (2);
  result rows: 0
  affect rows: 2

  update t set i = 3 where i = 2;
  result rows: 0
  affect rows: 1

  select * from t;
  result rows: 2
  affect rows: -1

  delete from t;
  result rows: 0
  affect rows: 2

Reviewers: jtolmer, tianx

Reviewed By: tianx

Subscribers: webscalesql-eng

Differential Revision: https://reviews.facebook.net/D49329
  • Loading branch information
lth authored and jtolmer committed Jan 5, 2016
1 parent dea28af commit ce95a09
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 16 deletions.
8 changes: 7 additions & 1 deletion include/mysql/plugin_audit.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

#define MYSQL_AUDIT_CLASS_MASK_SIZE 1

#define MYSQL_AUDIT_INTERFACE_VERSION 0x0302
#define MYSQL_AUDIT_INTERFACE_VERSION 0x0303


/*************************************************************************
Expand Down Expand Up @@ -67,6 +67,12 @@ struct mysql_event_general
long long query_id;
const char *database;
unsigned int database_length;
/* Added in version 0x303 */
unsigned long long affected_rows;
const char *connection_certificate;
unsigned int connection_certificate_length;
const char *query_attributes;
unsigned int query_attributes_length;
};


Expand Down
5 changes: 5 additions & 0 deletions include/mysql/plugin_audit.h.pp
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,11 @@
long long query_id;
const char *database;
unsigned int database_length;
unsigned long long affected_rows;
const char *connection_certificate;
unsigned int connection_certificate_length;
const char *query_attributes;
unsigned int query_attributes_length;
};
struct mysql_event_connection
{
Expand Down
5 changes: 5 additions & 0 deletions sql/sql_audit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,17 @@ static void general_class_handler(THD *thd, uint event_subtype, va_list ap)
event.general_query_length= va_arg(ap, unsigned int);
event.general_charset= va_arg(ap, struct charset_info_st *);
event.general_rows= (unsigned long long) va_arg(ap, ha_rows);
event.affected_rows= (long long) va_arg(ap, longlong);
event.general_sql_command= va_arg(ap, MYSQL_LEX_STRING);
event.general_host= va_arg(ap, MYSQL_LEX_STRING);
event.general_external_user= va_arg(ap, MYSQL_LEX_STRING);
event.general_ip= va_arg(ap, MYSQL_LEX_STRING);
event.database= va_arg(ap, const char *);
event.database_length= va_arg(ap, unsigned int);
event.connection_certificate= va_arg(ap, const char*);
event.connection_certificate_length= va_arg(ap, unsigned int);
event.query_attributes= va_arg(ap, const char*);
event.query_attributes_length= va_arg(ap, unsigned int);
event.query_id= (unsigned long long) thd->query_id;
event_class_dispatch(thd, MYSQL_AUDIT_GENERAL_CLASS, &event);
}
Expand Down
66 changes: 51 additions & 15 deletions sql/sql_audit.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,18 @@ void mysql_audit_general_log(THD *thd, const char *cmd, uint cmdlen,
MYSQL_LEX_STRING sql_command, ip, host, external_user;
MYSQL_LEX_STRING query={ (char *)query_str, query_len };
static MYSQL_LEX_STRING empty= { C_STRING_WITH_LEN("") };
ha_rows rows= 0;
ha_rows resultrows= 0;
longlong affectrows= 0;
int error_code= 0;
char user_buff[MAX_USER_HOST_SIZE + 1];
const char *user= user_buff;
uint userlen= make_user_name(thd, user_buff);
uint userlen, databaselen, certlen, queryattrlen;
const char *user, *database, *cert, *queryattr;
time_t time= (time_t) thd->start_time.tv_sec;
size_t databaselen;
const char *database;

if (thd)
{
user= user_buff;
userlen= make_user_name(thd, user_buff);
if (!query_len)
{
/* no query specified, fetch from THD */
Expand All @@ -112,23 +113,34 @@ void mysql_audit_general_log(THD *thd, const char *cmd, uint cmdlen,
sql_command.length= sql_statement_names[thd->lex->sql_command].length;
database= thd->db;
databaselen= thd->db_length;
cert= 0;
certlen= 0;
queryattr= 0;
queryattrlen= 0;
}
else
{
user= 0;
userlen= 0;
ip= empty;
host= empty;
external_user= empty;
sql_command= empty;
database= 0;
databaselen= 0;
cert= 0;
certlen= 0;
queryattr= 0;
queryattrlen= 0;
}
const CHARSET_INFO *clientcs= thd ? thd->variables.character_set_client
: global_system_variables.character_set_client;

mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, MYSQL_AUDIT_GENERAL_LOG,
error_code, time, user, userlen, cmd, cmdlen, query.str,
query.length, clientcs, rows, sql_command, host,
external_user, ip, database, databaselen);
query.length, clientcs, resultrows, affectrows,
sql_command, host, external_user, ip, database,
databaselen, cert, certlen, queryattr, queryattrlen);
}
#endif
}
Expand All @@ -155,12 +167,17 @@ void mysql_audit_general(THD *thd, uint event_subtype,
{
time_t time= my_time(0);
uint msglen= msg ? strlen(msg) : 0;
uint userlen, databaselen;
const char *user, *database;
uint userlen, databaselen, certlen, queryattrlen;
const char *user, *database, *cert, *queryattr;
char user_buff[MAX_USER_HOST_SIZE];
CSET_STRING query;
MYSQL_LEX_STRING ip, host, external_user, sql_command;
ha_rows rows;
// Result rows will hold the number of rows sent to the client.
ha_rows resultrows;
// Affected rows will hold the number of modified rows for DML statements.
// For SELECT statements, it is -1. For other statements, it is 0. See
// THD::get_row_count_func for details.
longlong affectrows;
static MYSQL_LEX_STRING empty= { C_STRING_WITH_LEN("") };


Expand All @@ -176,7 +193,16 @@ void mysql_audit_general(THD *thd, uint event_subtype,
query= thd->query_string;
user= user_buff;
userlen= make_user_name(thd, user_buff);
rows= thd->get_stmt_da()->current_row_for_warning();
if (event_subtype == MYSQL_AUDIT_GENERAL_STATUS)
{
affectrows= 0;
resultrows= 0;
}
else
{
affectrows= thd->get_row_count_func();
resultrows= thd->get_sent_row_count();
}
ip.str= (char *) thd->security_ctx->get_ip()->ptr();
ip.length= thd->security_ctx->get_ip()->length();
host.str= (char *) thd->security_ctx->get_host()->ptr();
Expand All @@ -187,6 +213,10 @@ void mysql_audit_general(THD *thd, uint event_subtype,
sql_command.length= sql_statement_names[thd->lex->sql_command].length;
database= thd->db;
databaselen= thd->db_length;
cert= 0;
certlen= 0;
queryattr= 0;
queryattrlen= 0;
}
else
{
Expand All @@ -196,16 +226,22 @@ void mysql_audit_general(THD *thd, uint event_subtype,
host= empty;
external_user= empty;
sql_command= empty;
rows= 0;
resultrows= 0;
affectrows= 0;
database= 0;
databaselen= 0;
cert= 0;
certlen= 0;
queryattr= 0;
queryattrlen= 0;
}

mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, event_subtype,
error_code, time, user, userlen, msg, msglen,
query.str(), query.length(), query.charset(), rows,
sql_command, host, external_user, ip, database,
databaselen);
query.str(), query.length(), query.charset(),
resultrows, affectrows, sql_command, host,
external_user, ip, database, databaselen,
cert, certlen, queryattr, queryattrlen);
}
#endif
}
Expand Down

0 comments on commit ce95a09

Please sign in to comment.