maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sachin!

Looks fine.

Two comments:
  1. optimization idea, see below
  2. add a test case that actually restores from the dump (and checks
     that it was restored correctly). mysqldump.test has examples,
     search for $MYSQL.

After that - please push!

On Dec 07, sachin wrote:
> revision-id: 78a1779f29ed8d2a96658583065270de8bdd14b4 (mariadb-10.3.2-61-g78a1779)
> parent(s): e8c8300a1c7e7e2b99e98c5c6986df69943cb661
> author: Sachin Setiya
> committer: Sachin Setiya
> timestamp: 2017-12-07 16:09:16 +0530
> message:
> 
> mysqldump fix for invisible column
> 
> Actually there are 2 issues in the case of invisible columns
> 
> 1st `select fields from t1` will have more fields then `select * from t1`.
> So instead of `select * from t1` we are using `select a,b,invisible from t1`
> these fields are supplied from `select fields from t1`.
> 
> 2nd We are using --complete-insert when we detect that this table is using
> invisible columns.
> 
> diff --git a/client/mysqldump.c b/client/mysqldump.c
> index a260065..818bc07 100644
> --- a/client/mysqldump.c
> +++ b/client/mysqldump.c
> @@ -2971,6 +2979,12 @@ static uint get_table_structure(char *table, char *db, char *table_type,
>        DBUG_RETURN(0);
>      }
>  
> +    while ((row= mysql_fetch_row(result)))
> +    {
> +     if (strlen(row[SHOW_EXTRA]) && strstr(row[SHOW_EXTRA],"INVISIBLE"))
> +       complete_insert= 1;
> +    }
> +    mysql_data_seek(result, 0);
>      /*
>        If write_data is true, then we build up insert statements for
>        the table's data. Note: in subsequent lines of code, this test
> @@ -3006,10 +3020,16 @@ static uint get_table_structure(char *table, char *db, char *table_type,
>          {
>            dynstr_append_checked(&insert_pat, ", ");
>          }
> -        init=1;
>          dynstr_append_checked(&insert_pat,
>                        quote_name(row[SHOW_FIELDNAME], name_buff, 0));
>        }
> +      if (init)
> +      {
> +        dynstr_append_checked(&select_field_names, ", ");
> +      }
> +      dynstr_append_checked(&select_field_names,
> +                    quote_name(row[SHOW_FIELDNAME], name_buff, 0));
> +      init=1;

I think, you can do it with just one loop:

     while ((row= mysql_fetch_row(result)))
     {
       if (strlen(row[SHOW_EXTRA]) && strstr(row[SHOW_EXTRA],"INVISIBLE"))
         complete_insert= 1;
       if (init)
         dynstr_append_checked(&select_field_names, ", ");
       dynstr_append_checked(&select_field_names,
                     quote_name(row[SHOW_FIELDNAME], name_buff, 0));
       init=1;
     }
     if (complete_insert)
       dynstr_append_checked(&insert_pat, select_field_names->str);

>      }
>      num_fields= mysql_num_rows(result);
>      mysql_free_result(result);

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx