Hi Sergei, thank you for looking at it. >>+ fn_format(raw_tblname, data_file.c_str(), "", "", MYF(MY_REPLACE_DIR | >>MY_REPLACE_EXT)); > >either use data_file.c_str() here and dbname.c_str() in >decode_fs_safe_name() above. Or dfile here and db above. Just for >consistency. Otherwise someone will think that there must've been a >valid reason for using db there and data_file.c_str() here. I did.
Agreed. I added more symmetry-consistency to this fragment. >> + if (!strcmp(default_charset, MYSQL_AUTODETECT_CHARSET_NAME)) >> + default_charset= (char *) my_default_csname(); >> + my_set_console_cp(default_charset); >> + default_charset_info= get_charset_by_csname(default_charset, >> MY_CS_PRIMARY, 0); > >Was it needed for a bug fix, or you just moved it as a generic >optimization / code cleanup? Sort of cleanup / optimization, yes. One could do without, but then decode_fs_safe_name() would have repeated unnecessary calls to "get_charset_by_csname(default_charset, MY_CS_PRIMARY, 0)" instead of a shorter / easier to read "default_charset_info". We'd need default_charset_info early enough, before table_load_params constructor, thus charset related initialization was moved here. >> + >> +--echo # MDEV-37483 mariadb-dump --dir doesn't convert db names > >three-line header, please Done. Thanks again, Wlad On Mon, Jan 12, 2026 at 10:33 PM Sergei Golubchik via developers <[email protected]> wrote: > > Hi, Vladislav, > > The fix is, of course, ok. > A couple of small code-related nitpicks, please, see below. > > On Jan 12, Vladislav Vaintroub wrote: > > revision-id: 4fb4fc5a59d (mariadb-11.8.4-22-g4fb4fc5a59d) > > parent(s): e7a5539927e > > author: Vladislav Vaintroub > > committer: Vladislav Vaintroub > > timestamp: 2026-01-12 07:57:47 +0100 > > message: > > > > MDEV-38498 mariadb-dump -dir doesn't convert database names > > > > Similar to MDEV-37483, use file name encoding for dbnames to create > > directories. Adapt mariadb-import to convert the names back. > > > > diff --git a/client/mysqlimport.cc b/client/mysqlimport.cc > > --- a/client/mysqlimport.cc > > +++ b/client/mysqlimport.cc > > @@ -104,6 +128,16 @@ struct table_load_params > > ddl_info(sql_text) > > { > > is_view= ddl_info.table_name.empty(); > > + /* Convert dbname from FS safe encoding if needed. */ > > + char decoded_name[FN_REFLEN]; > > + uint len= decode_fs_safe_name(db, decoded_name, sizeof(decoded_name)); > > + if (len) > > + dbname= std::string(decoded_name, len); > > + > > + char raw_tblname[FN_REFLEN]; > > + fn_format(raw_tblname, data_file.c_str(), "", "", MYF(MY_REPLACE_DIR | > > MY_REPLACE_EXT)); > > either use data_file.c_str() here and dbname.c_str() in > decode_fs_safe_name() above. Or dfile here and db above. Just for > consistency. Otherwise someone will think that there must've been a > valid reason for using db there and data_file.c_str() here. I did. > > > + len= decode_fs_safe_name(raw_tblname, decoded_name, > > sizeof(decoded_name)); > > + tablename= len ? decoded_name : raw_tblname; > > again, inconsistent with std::string(decoded_name, len) earlier, but > here you don't have a length of raw_tblname, at least there's a reason. > > > } > > int create_table_or_view(MYSQL *); > > int load_data(MYSQL *); > > @@ -879,9 +900,6 @@ static MYSQL *db_connect(char *host, char *database, > > > > if (opt_default_auth && *opt_default_auth) > > mysql_options(mysql, MYSQL_DEFAULT_AUTH, opt_default_auth); > > - if (!strcmp(default_charset,MYSQL_AUTODETECT_CHARSET_NAME)) > > - default_charset= (char *)my_default_csname(); > > - my_set_console_cp(default_charset); > > mysql_options(mysql, MYSQL_SET_CHARSET_NAME, default_charset); > > mysql_options(mysql, MYSQL_OPT_CONNECT_ATTR_RESET, 0); > > mysql_options4(mysql, MYSQL_OPT_CONNECT_ATTR_ADD, > > @@ -1273,6 +1291,12 @@ int main(int argc, char **argv) > > free_defaults(argv_to_free); > > return(1); > > } > > + > > + if (!strcmp(default_charset, MYSQL_AUTODETECT_CHARSET_NAME)) > > + default_charset= (char *) my_default_csname(); > > + my_set_console_cp(default_charset); > > + default_charset_info= get_charset_by_csname(default_charset, > > MY_CS_PRIMARY, 0); > > Was it needed for a bug fix, or you just moved it as a generic > optimization / code cleanup? > > > if (opt_use_threads > MAX_THREADS) > > { > > fatal_error("Too many connections, max value for --parallel is %d\n", > > diff --git a/mysql-test/main/mariadb-import.test > > b/mysql-test/main/mariadb-import.test > > --- a/mysql-test/main/mariadb-import.test > > +++ b/mysql-test/main/mariadb-import.test > > @@ -235,3 +235,41 @@ use test; > > --exec $MYSQL_IMPORT --dir $MYSQLTEST_VARDIR/tmp/dump --parallel=300 2>&1 > > > > --rmdir $MYSQLTEST_VARDIR/tmp/dump > > + > > +--echo # MDEV-37483 mariadb-dump --dir doesn't convert db names > > three-line header, please > otherwise ok, thanks. > > Regards, > Sergei > Chief Architect, MariaDB Server > and [email protected] > _______________________________________________ > developers mailing list -- [email protected] > To unsubscribe send an email to [email protected] _______________________________________________ developers mailing list -- [email protected] To unsubscribe send an email to [email protected]
