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@mariadb.org