← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 135d926d3: MDEV-6714 mysqldump slow with tables in big databases

 

Hi, Vicentiu!

Just one minor change, see the comment in get_actual_table_name().
After that please push.

Well done, thanks!

Regards,
Sergei

On Mar 19, vicentiu@xxxxxxxxxxx wrote:
> revision-id: 135d926d347443672626f1e655709fd5bf6dec80
> parent(s): 197afb413fcc9f06b5e5e6ef41ce980d108b354f
> committer: Vicențiu Ciorbaru
> branch nick: server
> timestamp: 2015-03-19 15:16:22 +0200
> message:
> 
> MDEV-6714 mysqldump slow with tables in big databases
> 
> mysqldump now attempts to make use of the INFORMATION_SCHEMA tables.
> If the table name is not found with a case sensitive search, it
> fallbacks to a case insensitive search.
> 
> ---
>  client/mysqldump.c | 90 ++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 71 insertions(+), 19 deletions(-)
> 
> diff --git a/client/mysqldump.c b/client/mysqldump.c
> index 2da4ce6..c265217 100644
> --- a/client/mysqldump.c
> +++ b/client/mysqldump.c
> @@ -3341,8 +3341,10 @@ static int dump_triggers_for_table(char *table_name, char *db_name)
>    /* Get list of triggers. */
>  
>    my_snprintf(query_buff, sizeof(query_buff),
> -              "SHOW TRIGGERS LIKE %s",
> -              quote_for_like(table_name, name_buff));
> +              "SELECT TRIGGER_NAME FROM INFORMATION_SCHEMA.TRIGGERS "
> +              "WHERE EVENT_OBJECT_SCHEMA = DATABASE() AND "
> +              "EVENT_OBJECT_TABLE = \"%s\"",
> +              table_name);

You still need to quote table_name, it might contain a double quote
inside. quote_names() almost does what you need, but you don't need a
check for MASK_ANSI_QUOTES. So you might either create a
quote_names_helper, again, or simply create a new function for this,
it's just a few lines anyway.

UPD: There are other similar places in mysqldump where it doesn't quote
table names, e.g. in get_table_structure(), for show_fields_stmt query.
So, please create a separate bug report for this and then it'll fix
everything at once.

>    if (mysql_query_with_error_report(mysql, &show_triggers_rs, query_buff))
>      goto done;
> @@ -4695,29 +4697,37 @@ static my_bool dump_all_views_in_db(char *database)
>  
>  
>  /*
> -  get_actual_table_name -- executes a SHOW TABLES LIKE '%s' to get the actual
> -  table name from the server for the table name given on the command line.
> -  we do this because the table name given on the command line may be a
> -  different case (e.g.  T1 vs t1)
> -
> -  RETURN
> -    pointer to the table name
> -    0 if error
> +  See get_actual_table_name. Used to retrieve the correct table name
> +  from the database schema.
>  */
> -
> -static char *get_actual_table_name(const char *old_table_name, MEM_ROOT *root)
> +static char *get_actual_table_name_helper(const char *old_table_name,
> +                                          my_bool case_sensitive,
> +                                          MEM_ROOT *root)
>  {
>    char *name= 0;
>    MYSQL_RES  *table_res;
>    MYSQL_ROW  row;
>    char query[50 + 2*NAME_LEN];
>    char show_name_buff[FN_REFLEN];
> -  DBUG_ENTER("get_actual_table_name");
> +  DBUG_ENTER("get_actual_table_name_helper");
>  
>    /* Check memory for quote_for_like() */
>    DBUG_ASSERT(2*sizeof(old_table_name) < sizeof(show_name_buff));
> -  my_snprintf(query, sizeof(query), "SHOW TABLES LIKE %s",
> -              quote_for_like(old_table_name, show_name_buff));
> +
> +  if (case_sensitive)
> +  {
> +    DBUG_PRINT("info", ("case sensitive search"));
> +    my_snprintf(query, sizeof(query),
> +                "SELECT table_name FROM INFORMATION_SCHEMA.TABLES "
> +                "WHERE table_schema = DATABASE() AND table_name = \"%s\"",
> +                old_table_name);
> +  }
> +  else
> +  {
> +    DBUG_PRINT("info", ("case insensitive search"));
> +    my_snprintf(query, sizeof(query), "SHOW TABLES LIKE %s",
> +                quote_for_like(old_table_name, show_name_buff));
> +  }
>  
>    if (mysql_query_with_error_report(mysql, 0, query))
>      return NullS;
> @@ -4742,6 +4752,46 @@ static char *get_actual_table_name(const char *old_table_name, MEM_ROOT *root)
>    DBUG_RETURN(name);
>  }
>  
> +/*
> +  get_actual_table_name -- executes a SELECT .. FROM I_S.tables to check
> +  if the table name given on the command line matches the one in the database.
> +  If the table is not found, it falls back to a slower SHOW TABLES LIKE '%s' to
> +  get the actual table name from the server.
> +
> +  We do this because the table name given on the command line may be a
> +  different case (e.g.  T1 vs t1), but checking this takes a long time
> +  when there are many tables present.
> +
> +  RETURN
> +    pointer to the table name
> +    0 if error
> +*/
> +
> +static char *get_actual_table_name(const char *old_table_name, MEM_ROOT *root)
> +{
> +  char *name= 0;
> +  MYSQL_RES  *table_res;
> +  MYSQL_ROW  row;
> +  int lower_case_table_names;
> +  const char *show_var_query = "SHOW VARIABLES LIKE 'lower_case_table_names'";
> +  DBUG_ENTER("get_actual_table_name");
> +
> +  if (mysql_query_with_error_report(mysql, &table_res, show_var_query))
> +    return NullS;

I'd prefer if you do it only once, not for every get_actual_table_name()
call.

> +
> +  if ((row= mysql_fetch_row(table_res)))
> +  {
> +    lower_case_table_names= atoi(row[1]);
> +    mysql_free_result(table_res);
> +  }
> +
> +  name= get_actual_table_name_helper(old_table_name, TRUE, root);
> +  if (!name && !lower_case_table_names)
> +    name= get_actual_table_name_helper(old_table_name, FALSE, root);
> +  DBUG_RETURN(name);
> +}
> +
> +
>  
>  static int dump_selected_tables(char *db, char **table_names, int tables)
>  {