June 2019 - developers - lists.mariadb.org

Re: [Maria-developers] b4cdf20dd15: MDEV-17591 Create MariaDB named commands/symlinks
by Sergei Golubchik 14 Jun '19

14 Jun '19

Hi, Rasmus! On Jun 14, Rasmus Johansson wrote: > revision-id: b4cdf20dd15 (mariadb-10.4.5-44-gb4cdf20dd15) > parent(s): 8e3a4be45c5 > author: Rasmus Johansson <rasmus(a)mariadb.com> > committer: Rasmus Johansson <rasmus(a)mariadb.com> > timestamp: 2019-06-14 14:55:20 +0000 > message: > > MDEV-17591 Create MariaDB named commands/symlinks > > diff --git a/cmake/symlinks.cmake b/cmake/symlinks.cmake > new file mode 100644 > index 00000000000..… [View More]32cf24e3e3b > --- /dev/null > +++ b/cmake/symlinks.cmake > @@ -0,0 +1,76 @@ > +# MariaDB names for executables > +list(APPEND MARIADB_SYMLINK_NAMES "mysql" "mariadb") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqlaccess" "mariadb-access") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqladmin" "mariadb-admin") > +list(APPEND MARIADB_SYMLINK_NAMES "mariabackup" "mariadb-backup") ... > +# Add MariaDB symlinks > +macro(CREATE_MARIADB_SYMLINK src comp) > + # Find the MariaDB name for executable > + list(FIND MARIADB_SYMLINK_NAMES ${src} _index) > + > + if (${_index} GREATER -1) > + MATH(EXPR _index "${_index}+1") > + list(GET MARIADB_SYMLINK_NAMES ${_index} _name) > + set(mariadbname ${_name}) > + endif() > + > + if (mariadbname) > + set(dest ${mariadbname}) > + set(symlink_install_dir ${INSTALL_BINDIR}) > + > + # adjust install location if needed > + if(${dest} MATCHES "mariadb-install-db") > + set(symlink_install_dir ${INSTALL_SCRIPTDIR}) > + endif() I still think it's very fragile and difficult to maintain. Why not to pass the destination as a third argument to CREATE_MARIADB_SYMLINK? As far as I can see, everywhere where you use CREATE_MARIADB_SYMLINK the destination is available. The rest is okay. I'd still suggest to simplify by splitting the list in two, like macro(REGISTER_SYMLINK from to) list(APPEND MARIADB_SYMLINK_FROMS from) list(APPEND MARIADB_SYMLINK_TOS to) endmacro() REGISTER_SYMLINK(mysql mariadb) REGISTER_SYMLINK(mysqladmin mariadb-access) ...etc... Then you won't need to do math on indexes, you simply search in MARIADB_SYMLINK_FROMS and get the same element in MARIADB_SYMLINK_TOS. Also you will need to search only in MARIADB_SYMLINK_FROMS, not like now, when you search in the complete list, risking to get spurious matches on mariadb* names. Same in manpages, you'll only iterate over MARIADB_SYMLINK_FROMS list. > + > + CREATE_MARIADB_SYMLINK_IN_DIR(${src} ${dest} ${symlink_install_dir} ${comp}) > + endif() > +endmacro(CREATE_MARIADB_SYMLINK) Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org [View Less]

1 0

Re: [Maria-developers] e35fb718eae: MDEV-17591 Create MariaDB named commands/symlinks
by Sergei Golubchik 14 Jun '19

14 Jun '19

Hi, Rasmus! On Jun 13, Rasmus Johansson wrote: > revision-id: e35fb718eae (mariadb-10.4.5-44-ge35fb718eae) > parent(s): 8e3a4be45c5 > author: Rasmus Johansson <razze(a)iki.fi> > committer: Rasmus Johansson <razze(a)iki.fi> Is that an address you want to have in git history? You already have two, just making sure the third one is intentional. > timestamp: 2019-06-13 13:20:40 +0000 > message: > > MDEV-17591 Create MariaDB named commands/symlinks > > … [View More]diff --git a/cmake/install_macros.cmake b/cmake/install_macros.cmake > index f28720e97bc..e49f296ed75 100644 > --- a/cmake/install_macros.cmake > +++ b/cmake/install_macros.cmake > @@ -111,8 +111,10 @@ FUNCTION(INSTALL_SCRIPT) > ENDIF() > > INSTALL(PROGRAMS ${script} DESTINATION ${ARG_DESTINATION} ${COMP}) > + string(REPLACE "${CMAKE_BINARY_DIR}/scripts/" "" dest ${script}) better not to assume that the script is in ${CMAKE_BINARY_DIR}/scripts/ I'd use get_filename_component() here. Like get_filename_component(dest "${scripts}" NAME) > + CREATE_MARIADB_SYMLINK(${dest}) > > - INSTALL_MANPAGE(${script}) > + INSTALL_MANPAGE(${dest}) > ENDFUNCTION() > > > diff --git a/cmake/mysql_add_executable.cmake b/cmake/mysql_add_executable.cmake > index d3a888f9a75..ad7320cd193 100644 > --- a/cmake/mysql_add_executable.cmake > +++ b/cmake/mysql_add_executable.cmake > @@ -63,6 +63,9 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) > UNSET(EXCLUDE_FROM_ALL) > ENDIF() > ADD_EXECUTABLE(${target} ${WIN32} ${MACOSX_BUNDLE} ${EXCLUDE_FROM_ALL} ${sources}) > + # create mariadb named symlink > + CREATE_MARIADB_SYMLINK(${target}) > + > # tell CPack where to install > IF(NOT ARG_EXCLUDE_FROM_ALL) > IF(NOT ARG_DESTINATION) > diff --git a/cmake/symlinks.cmake b/cmake/symlinks.cmake > new file mode 100644 > index 00000000000..0d0c637df5a > --- /dev/null > +++ b/cmake/symlinks.cmake > @@ -0,0 +1,74 @@ > +# MariaDB names for executables > +list(APPEND MARIADB_SYMLINK_NAMES "mysql" "mariadb") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqlaccess" "mariadb-access") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqladmin" "mariadb-admin") > +list(APPEND MARIADB_SYMLINK_NAMES "mariabackup" "mariadb-backup") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqlbinlog" "mariadb-binlog") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqlcheck" "mariadb-check") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_client_test_embedded" "mariadb-client-test-embedded") > +list(APPEND MARIADB_SYMLINK_NAMES "mariadb_config" "mariadb-config") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_convert_table_format" "mariadb-convert-table-format") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqldump" "mariadb-dump") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqldumpslow" "mariadb-dumpslow") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_embedded" "mariadb-embedded") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_find_rows" "mariadb-find-rows") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_fix_extensions" "mariadb-fix-extensions") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqlhotcopy" "mariadb-hotcopy") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqlimport" "mariadb-import") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_install_db" "mariadb-install-db") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_ldb" "mariadb-ldb") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_plugin" "mariadb-plugin") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_secure_installation" "mariadb-secure-installation") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_setpermission" "mariadb-setpermission") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqlshow" "mariadb-show") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqlslap" "mariadb-slap") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqltest" "mariadb-test") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqltest_embedded" "mariadb-test-embedded") > +list(APPEND MARIADB_SYMLINK_NAMES "mytop" "mariadb-top") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_tzinfo_to_sql" "mariadb-tzinfo-to-sql") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_upgrade" "mariadb-upgrade") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_upgrade_service" "mariadb-upgrade-service") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_upgrade_wizard" "mariadb-upgrade-wizard") > +list(APPEND MARIADB_SYMLINK_NAMES "mysql_waitpid" "mariadb-waitpid") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqld" "mariadbd") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqld_multi" "mariadbd-multi") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqld_safe" "mariadbd-safe") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqld_safe_helper" "mariadbd-safe-helper") > + > +# Add MariaDB symlinks > +macro(CREATE_MARIADB_SYMLINK src) do you still skip Windows, as in the previous commit? I couldn't find it in this one. > + # Find the MariaDB name for executable > + list(FIND MARIADB_SYMLINK_NAMES ${src} _index) > + > + if (${_index} GREATER -1) > + MATH(EXPR _index "${_index}+1") > + list(GET MARIADB_SYMLINK_NAMES ${_index} _name) > + MESSAGE(${_name}) I suspect this was a debugging aid that you forgot to remove > + set(mariadbname ${_name}) > + endif() > + > + if (mariadbname) > + set(dest ${mariadbname}) > + message("src: ${src}") > + message("dest: ${dest}") > + message("CMAKE_CURRENT_BINARY_DIR: ${CMAKE_CURRENT_BINARY_DIR}") > + message("INSTALL_BINDIR: ${INSTALL_BINDIR}") and that too > + > + add_custom_target( > + symlink_${dest} ALL > + DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${dest} > + ) > + > + add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${dest} POST_BUILD > + COMMAND ${CMAKE_COMMAND} -E create_symlink ${src} ${dest} > + COMMENT "mklink ${src} -> ${dest}") > + > + set(symlink_install_dir ${INSTALL_BINDIR}) > + # adjust install location if needed > + if(${dest} MATCHES "mariadb-install-db") > + set(symlink_install_dir ${INSTALL_SCRIPTDIR}) > + endif() this is very fragile. better to take the path from the ${src} also with get_filename_component(). > + > + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${dest} DESTINATION ${symlink_install_dir} COMPONENT symlinks) > + endif() > +endmacro(CREATE_MARIADB_SYMLINK) > diff --git a/debian/mariadb-client-10.4.links b/debian/mariadb-client-10.4.links > index 4a504969246..b93d2d7f8fd 100644 > --- a/debian/mariadb-client-10.4.links > +++ b/debian/mariadb-client-10.4.links > @@ -1,6 +1,44 @@ > +usr/bin/mysql_find_rows usr/bin/mariadb-find-rows > +usr/bin/mysql_fix_extensions usr/bin/mariadb-fix-extenstions > +usr/bin/mysql_plugin usr/bin/mariadb-plugin > +usr/bin/mysql_waitpid usr/bin/mariadb-waitpid > +usr/bin/mysqlaccess usr/bin/mariadb-access > +usr/bin/mysqladmin usr/bin/mariadb-admin > +usr/bin/mysqlanalyze usr/bin/mariadb-analyze > +usr/bin/mysqlbinlog usr/bin/mariadb-binlog > +usr/bin/mysqlcheck usr/bin/mariadb-analyze > +usr/bin/mysqlcheck usr/bin/mariadb-optimize > +usr/bin/mysqlcheck usr/bin/mariadb-repair > usr/bin/mysqlcheck usr/bin/mysqlanalyze > usr/bin/mysqlcheck usr/bin/mysqloptimize > usr/bin/mysqlcheck usr/bin/mysqlrepair > +usr/bin/mysqldump usr/bin/mariadb-dump > +usr/bin/mysqldumpslow usr/bin/mariadb-dump-slow > +usr/bin/mysqlimport usr/bin/mariadb-import > +usr/bin/mysqloptimize usr/bin/mariadb-optimize > +usr/bin/mysqlrepair usr/bin/mariadb-repair > +usr/bin/mysqlreport usr/bin/mariadb-report > +usr/bin/mysqlslap usr/bin/mariadb-slap > +usr/bin/mysqltest usr/bin/mariadb-test > +usr/share/man/man1/mysql_find_rows.1.gz usr/share/man/man1/mariadb-find-rows.1.gz > +usr/share/man/man1/mysql_fix_extensions.1.gz usr/share/man/man1/mariadb-fix-extensions.1.gz manpages aren't symlinked in rpms or bintars. and symlinks depends only on MariaDB-client, while for example mysql_install_db is in the server package. may be just put symlinks into the same package as symlink targets? just like with debs? > +usr/share/man/man1/mysql_plugin.1.gz usr/share/man/man1/mariadb-plugin.1.gz > +usr/share/man/man1/mysql_waitpid.1.gz usr/share/man/man1/mariadb-waitpid.1.gz > +usr/share/man/man1/mysqlaccess.1.gz usr/share/man/man1/mariadb-access.1.gz > +usr/share/man/man1/mysqladmin.1.gz usr/share/man/man1/mariadb-admin.1.gz > +usr/share/man/man1/mysqlanalyze.1.gz usr/share/man/man1/mariadb-analyze.1.gz > +usr/share/man/man1/mysqlbinlog.1.gz usr/share/man/man1/mariadb-binlog.1.gz > +usr/share/man/man1/mysqlcheck.1.gz usr/share/man/man1/mariadb-analyze.1.gz > +usr/share/man/man1/mysqlcheck.1.gz usr/share/man/man1/mariadb-optimize.1.gz > +usr/share/man/man1/mysqlcheck.1.gz usr/share/man/man1/mariadb-repair.1.gz > usr/share/man/man1/mysqlcheck.1.gz usr/share/man/man1/mysqlanalyze.1.gz > usr/share/man/man1/mysqlcheck.1.gz usr/share/man/man1/mysqloptimize.1.gz > usr/share/man/man1/mysqlcheck.1.gz usr/share/man/man1/mysqlrepair.1.gz > +usr/share/man/man1/mysqldump.1.gz usr/share/man/man1/mariadb-dump.1.gz > +usr/share/man/man1/mysqldumpslow.1.gz usr/share/man/man1/mariadb-dump-slow.1.gz > +usr/share/man/man1/mysqlimport.1.gz usr/share/man/man1/mariadb-import.1.gz > +usr/share/man/man1/mysqloptimize.1.gz usr/share/man/man1/mariadb-optimize.1.gz > +usr/share/man/man1/mysqlrepair.1.gz usr/share/man/man1/mariadb-repair.1.gz > +usr/share/man/man1/mysqlreport.1.gz usr/share/man/man1/mariadb-report.1.gz > +usr/share/man/man1/mysqlslap.1.gz usr/share/man/man1/mariadb-slap.1.gz > +usr/share/man/man1/mysqltest.1.gz usr/share/man/man1/mariadb-test.1.gz > diff --git a/debian/mariadb-client-core-10.4.links b/debian/mariadb-client-core-10.4.links > new file mode 100644 > index 00000000000..4c5ff23b38b > --- /dev/null > +++ b/debian/mariadb-client-core-10.4.links > @@ -0,0 +1,4 @@ > +usr/bin/mysql usr/bin/mariadb > +usr/bin/mysqlcheck usr/bin/mariadb-check > +usr/share/man/man1/mysql.1.gz usr/share/man/man1/mariadb.1.gz > +usr/share/man/man1/mysqlcheck.1.gz usr/share/man/man1/mariadb-check.1.gz > diff --git a/debian/mariadb-server-10.4.install b/debian/mariadb-server-10.4.install > index a9cbbfc8a99..8b4db99a421 100644 > --- a/debian/mariadb-server-10.4.install > +++ b/debian/mariadb-server-10.4.install > @@ -87,9 +87,9 @@ usr/share/man/man1/replace.1 > usr/share/man/man1/resolve_stack_dump.1 > usr/share/man/man1/resolveip.1 > usr/share/man/man1/wsrep_sst_common.1 > +usr/share/man/man1/wsrep_sst_mariabackup.1 > usr/share/man/man1/wsrep_sst_mysqldump.1 > usr/share/man/man1/wsrep_sst_rsync.1 > -usr/share/man/man1/wsrep_sst_mariabackup.1 > usr/share/mysql/errmsg-utf8.txt > usr/share/mysql/fill_help_tables.sql > usr/share/mysql/maria_add_gis_sp_bootstrap.sql > diff --git a/debian/mariadb-server-10.4.links b/debian/mariadb-server-10.4.links > new file mode 100644 > index 00000000000..65e0a6650d5 > --- /dev/null > +++ b/debian/mariadb-server-10.4.links > @@ -0,0 +1,18 @@ > +usr/bin/mysql_convert_table_format usr/bin/mariadb-convert-table-format > +usr/bin/mysql_secure_installation usr/bin/mariadb-secure-installation > +usr/bin/mysql_setpermission usr/bin/mariadb-setpermission > +usr/bin/mysql_tzinfo_to_sql usr/bin/mariadb-tzinfo-to-sql > +usr/bin/mysqld_multi usr/bin/mariadb-multi > +usr/bin/mysqld_safe usr/bin/mariadb-safe > +usr/bin/mysqld_safe_helper usr/bin/mariadb-safe-helper > +usr/bin/mysqlhotcopy usr/bin/mariadb-hotcopy > +usr/bin/mysqlshow usr/bin/mariadb-show > +usr/share/man/man1/mysql_convert_table_format.1.gz usr/share/man/man1/mariadb-convert-table-format.1.gz > +usr/share/man/man1/mysql_secure_installation.1.gz usr/share/man/man1/mariadb-secure-installation.1.gz > +usr/share/man/man1/mysql_setpermission.1.gz usr/share/man/man1/mariadb-setpermission.1.gz > +usr/share/man/man1/mysql_tzinfo_to_sql.1.gz usr/share/man/man1/mariadb-tzinfo-to-sql.1.gz > +usr/share/man/man1/mysqld_multi.1.gz usr/share/man/man1/mariadb-multi.1.gz > +usr/share/man/man1/mysqld_safe.1.gz usr/share/man/man1/mariadb-safe.1.gz > +usr/share/man/man1/mysqld_safe_helper.1.gz usr/share/man/man1/mariadb-safe-helper.1.gz > +usr/share/man/man1/mysqlhotcopy.1.gz usr/share/man/man1/mariadb-hotcopy.1.gz > +usr/share/man/man1/mysqlshow.1.gz usr/share/man/man1/mariadb-show.1.gz > diff --git a/debian/mariadb-server-core-10.4.links b/debian/mariadb-server-core-10.4.links > new file mode 100644 > index 00000000000..90ed9ee01cf > --- /dev/null > +++ b/debian/mariadb-server-core-10.4.links > @@ -0,0 +1,4 @@ > +usr/bin/mysql_install_db usr/bin/mariadb-install-db > +usr/bin/mysql_upgrade usr/bin/mariadb-upgrade > +usr/share/man/man1/mysql_install_db.1.gz usr/share/man/man1/mariadb-install-db.1.gz > +usr/share/man/man1/mysql_upgrade.1.gz usr/share/man/man1/mariadb-upgrade-db.1.gz > diff --git a/scripts/CMakeLists.txt b/scripts/CMakeLists.txt > index 3d31cda4ceb..7be46ac1985 100644 > --- a/scripts/CMakeLists.txt > +++ b/scripts/CMakeLists.txt > @@ -332,6 +332,7 @@ ELSE() > COMPONENT ${${file}_COMPONENT} > ) > ENDFOREACH() > + > SET (wsrep_sst_rsync_wan ${CMAKE_CURRENT_BINARY_DIR}/wsrep_sst_rsync_wan) > ADD_CUSTOM_COMMAND( > OUTPUT ${wsrep_sst_rsync_wan} > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org [View Less]

2 1

[Maria-developers] GSoC [MDEV-5092]
by Мирослав Коберский 14 Jun '19

14 Jun '19

Hello, I'm working in implementing UPDATE .. RETURNING. For beginning i'm working on single-table update. I've made parser to parse RETURNING clause in UPDATE. There was a problem that columns that should be updated and columns that should be returned were stored in the same list in lex named item_list. I'm trying now to divide them. Not sure that swapping lists two times during parsing between each other is the best idea. I've implemented primitive returning of result set every time doing an … [View More]

2 1

Re: [Maria-developers] 030c299f76f: MDEV-17591 Create MariaDB named commands/symlinks
by Sergei Golubchik 13 Jun '19

13 Jun '19

Hi, Rasmus! On Jun 11, Rasmus Johansson wrote: > revision-id: 030c299f76f (mariadb-10.4.5-33-g030c299f76f) > parent(s): 973b281e599 > author: Rasmus Johansson <razze(a)iki.fi> > committer: Rasmus Johansson <razze(a)iki.fi> Is that an address you want to have in git history? You already have two, just making sure the third one is intentional. > timestamp: 2019-06-09 07:33:41 +0000 > message: > > MDEV-17591 Create MariaDB named commands/symlinks > > … [View More]diff --git a/cmake/symlinks.cmake b/cmake/symlinks.cmake > new file mode 100644 > index 00000000000..da38d2c28d5 > --- /dev/null > +++ b/cmake/symlinks.cmake > @@ -0,0 +1,8 @@ > +# Add MariaDB symlinks > +MACRO(CREATE_MARIADB_SYMLINK binaryname symlinkname) > + IF (EXISTS ${CMAKE_CURRENT_BINARY_DIR}/${binaryname} AND NOT WIN32) > + install(CODE "execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink ${binaryname} ${CMAKE_CURRENT_BINARY_DIR}/${symlinkname})" COMPONENT symlinks) > + install(CODE "message(\"-- Created symlink: ${symlinkname} -> ${binaryname}\")" COMPONENT symlinks) > + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${symlinkname} DESTINATION ${INSTALL_BINDIR} COMPONENT symlinks) > + ENDIF() > +ENDMACRO(CREATE_MARIADB_SYMLINK) Hmm, that's very weird. 1. you test whether a binary exists _at configure time_ but the binary is only created during the build. So, the test is always false, unless you re-run cmake after the build, which is not a normal build procedure. 2. you create a symlink _at install time_ but in a CMAKE_CURRENT_BINARY_DIR. This is strange decision. Normally, build step creates files in CMAKE_CURRENT_BINARY_DIR, and install step creates files in INSTALL_BINDIR. Basically, to fix that you should either create symlinks as a new build target that dependents on binaries being created first. And then install these symlinks normally. Or create symlinks during install time directly at the correct install location, not in the binary dir. > diff --git a/debian/mariadb-client-10.4.links b/debian/mariadb-client-10.4.links > index 4a504969246..b93d2d7f8fd 100644 > --- a/debian/mariadb-client-10.4.links > +++ b/debian/mariadb-client-10.4.links > @@ -1,6 +1,44 @@ > +usr/bin/mysql_find_rows usr/bin/mariadb-find-rows > +usr/bin/mysql_fix_extensions usr/bin/mariadb-fix-extenstions > +usr/bin/mysql_plugin usr/bin/mariadb-plugin Why there's a separate rpm with symlinks, but no separate deb with symlinks (they're direcly in the client deb) ? Why would you want different approaches for deb and rpm? > +usr/bin/mysqld_multi usr/bin/mariadb-multi > +usr/bin/mysqld_safe usr/bin/mariadb-safe > +usr/bin/mysqld_safe_helper usr/bin/mariadb-safe-helper Also you seem to symlink a lot more files in deb than in rpm or in tar.gz. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org [View Less]

2 1

Re: [Maria-developers] 66f931bcd69: MDEV-18727 improve DML operation of System Versioning
by Sergei Golubchik 12 Jun '19

12 Jun '19

Hi, Aleksey! Looks ok, thanks! No requests to change anything, but still a couple of "why" questions, see below On Jun 07, Aleksey Midenkov wrote: > revision-id: 66f931bcd69 (mariadb-10.3.12-100-g66f931bcd69) > parent(s): f4484dfdbf2 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2019-03-26 15:04:52 +0300 > message: > > MDEV-18727 improve DML operation of System Versioning > MDEV-18957 … [View More]UPDATE with LIMIT clause is wrong for versioned partitioned tables > > UPDATE, DELETE: replace linear search of current/historical records > with vers_setup_conds(). > > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index b8ed887b52d..a00cc2a8fb0 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -4975,8 +4975,10 @@ mysql_execute_command(THD *thd) > { > result= new (thd->mem_root) multi_delete(thd, aux_tables, > lex->table_count); > - if (unlikely(result)) > + if (likely(result)) heh, good catch > { > + if (unlikely(select_lex->vers_setup_conds(thd, aux_tables))) > + goto multi_delete_error; > res= mysql_select(thd, > select_lex->get_table_list(), > select_lex->with_wild, > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 6915d4d23ca..28b54e0b3b1 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -779,20 +781,35 @@ int SELECT_LEX::vers_setup_conds(THD *thd, TABLE_LIST *tables) > } > } > > + bool is_select= false; > + switch (thd->lex->sql_command) > + { > + case SQLCOM_SELECT: > + case SQLCOM_INSERT_SELECT: > + case SQLCOM_REPLACE_SELECT: > + case SQLCOM_DELETE_MULTI: > + case SQLCOM_UPDATE_MULTI: > + is_select= true; > + default: > + break; > + } > + > for (table= tables; table; table= table->next_local) > { > - if (!table->table || !table->table->versioned()) > + if (!table->table || table->is_view() || !table->table->versioned()) why? > continue; > > vers_select_conds_t &vers_conditions= table->vers_conditions; > > #ifdef WITH_PARTITION_STORAGE_ENGINE > - /* > - if the history is stored in partitions, then partitions > - themselves are not versioned > - */ > - if (table->partition_names && table->table->part_info->vers_info) > + Vers_part_info *vers_info; > + if (is_select && table->table->part_info && > + (vers_info= table->table->part_info->vers_info)) > + { > + if (table->partition_names) Why is that? You don't seem to use vers_info anywhere. Nor I see a reason for two nested if()'s intead of one, as before (with an additional `is_select &&` condition) > { > + /* If the history is stored in partitions, then partitions > + themselves are not versioned. */ > if (vers_conditions.is_set()) > { > my_error(ER_VERS_QUERY_IN_PARTITION, MYF(0), table->alias.str); Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org [View Less]

2 1

Re: [Maria-developers] [Commits] 5bfc8128c83: MDEV-8874 Replication filters configured in my.cnf are ignored if slave reset and reconfigured
by andrei.elkin＠pp.inet.fi 12 Jun '19

12 Jun '19

Sachin, hello. The patch is alright. Thanks for a good piece of work! Andrei > revision-id: 5bfc8128c8381e9347589aa0e27cd4fbd66ef19d (mariadb-10.1.39-57-g5bfc8128c83) > parent(s): 2e0fb93e6f3359ffe146f309e32c59922400b7c4 > author: Sachin > committer: Sachin > timestamp: 2019-06-12 15:58:44 +0530 > message: > > MDEV-8874 Replication filters configured in my.cnf are ignored if slave reset and reconfigured > > Don't delete the rpl_filter on RESET SLAVE. >

1 0

Re: [Maria-developers] [Commits] 321a43d9ac5: MDEV-8874 Replication filters configured in my.cnf are ignored if slave reset and reconfigured
by andrei.elkin＠pp.inet.fi 12 Jun '19

12 Jun '19

Sachin, salute! The patch looks good, but I think it misses some verbosity in few places, please read on. > revision-id: 321a43d9ac586034790407dc519c72e90b3bff81 (mariadb-10.1.39-54-g321a43d9ac5) > parent(s): b003b0c934cf6b59358e31144c4f69cf34622fb8 > author: Sachin > committer: Sachin > timestamp: 2019-06-06 12:03:45 +0530 > message: > > MDEV-8874 Replication filters configured in my.cnf are ignored if slave reset and reconfigured > > Don't delete the rpl_filter … [View More]on RESET SLAVE. > > --- > mysql-test/lib/My/Config.pm | 8 +- > mysql-test/suite/multi_source/mdev-8874.cnf | 25 +++++ > mysql-test/suite/multi_source/mdev-8874.result | 93 +++++++++++++++++++ > mysql-test/suite/multi_source/mdev-8874.test | 123 +++++++++++++++++++++++++ > sql/rpl_mi.cc | 2 - > 5 files changed, 248 insertions(+), 3 deletions(-) > > diff --git a/mysql-test/lib/My/Config.pm b/mysql-test/lib/My/Config.pm > index 12647edf0a4..349a397ed6d 100644 > --- a/mysql-test/lib/My/Config.pm > +++ b/mysql-test/lib/My/Config.pm > @@ -327,7 +327,13 @@ sub new { > # Skip comment > next; > } > - > + > + # Replication Filter > + elsif ( $line =~ /^([\w]+.[\w]+)\s*=\s*(.*)\s*/){ > + my $option= $1; > + my $value= $2; > + $self->insert($group_name, $option, $value); > + } What was a problem at the above point? [Anything that you think may not be straightforward for a patch reader is always worth to highlight in the description.] > else { > croak "Unexpected line '$line' found in '$path'"; > } > diff --git a/mysql-test/suite/multi_source/mdev-8874.cnf b/mysql-test/suite/multi_source/mdev-8874.cnf > new file mode 100644 > index 00000000000..01da70cbaa0 > --- /dev/null > +++ b/mysql-test/suite/multi_source/mdev-8874.cnf > @@ -0,0 +1,25 @@ > +!include my.cnf > + > +[mysqld.1] > +log-bin > +log-slave-updates > + > +[mysqld.2] > +log-bin > +log-slave-updates > + > +[mysqld.3] > +log-bin > +log-slave-updates > + > +[mysqld.4] > +server-id=4 > +log-bin=server4-bin > +log-slave-updates > +m1.replicate_ignore_table='a.t1' > +m2.replicate_ignore_table='b.t1' > +replicate_ignore_table='c.t1' > + > +[ENV] > +SERVER_MYPORT_4= @mysqld.4.port > +SERVER_MYSOCK_4= @mysqld.4.socket > diff --git a/mysql-test/suite/multi_source/mdev-8874.result b/mysql-test/suite/multi_source/mdev-8874.result > new file mode 100644 > index 00000000000..6fb364a3d2d > --- /dev/null > +++ b/mysql-test/suite/multi_source/mdev-8874.result > @@ -0,0 +1,93 @@ > +create database a; > +use a; > +create table t1(a int); > +insert into t1 values(1); > +create table t2(a int); > +insert into t2 values(1); > +create database b; > +use b; > +create table t1(a int); > +insert into t1 values(1); > +create table t2(a int); > +insert into t2 values(1); > +create database c; > +use c; > +create table t1(a int); > +insert into t1 values(1); > +create table t2(a int); > +insert into t2 values(1); > +change master 'm1' to master_port=MYPORT_1 , master_host='127.0.0.1', master_user='root'; > +change master 'm2' to master_port=MYPORT_2 , master_host='127.0.0.1', master_user='root'; > +change master to master_port=MYPORT_3 , master_host='127.0.0.1', master_user='root'; > +start all slaves; > +set default_master_connection = 'm1'; > +include/wait_for_slave_to_start.inc > +set default_master_connection = 'm2'; > +include/wait_for_slave_to_start.inc > +set default_master_connection = ''; > +include/wait_for_slave_to_start.inc > +use a; > +show tables; > +Tables_in_a > +t2 > +use b; > +show tables; > +Tables_in_b > +t2 > +use c; > +show tables; > +Tables_in_c > +t2 > +#TEST > +STOP ALL SLAVES; > +Warnings: > +Note 1938 SLAVE 'm2' stopped > +Note 1938 SLAVE '' stopped > +Note 1938 SLAVE 'm1' stopped > +RESET SLAVE 'm1' ALL ; > +RESET SLAVE 'm2' ALL ; > +RESET SLAVE ALL ; > +drop database a; > +drop database b; > +drop database c; > +change master 'm1' to master_port=MYPORT_1 , master_host='127.0.0.1', master_user='root'; > +change master 'm2' to master_port=MYPORT_2 , master_host='127.0.0.1', master_user='root'; > +change master to master_port=MYPORT_3 , master_host='127.0.0.1', master_user='root'; > +start all slaves; > +Warnings: > +Note 1937 SLAVE 'm2' started > +Note 1937 SLAVE '' started > +Note 1937 SLAVE 'm1' started > +set default_master_connection = 'm1'; > +include/wait_for_slave_to_start.inc > +set default_master_connection = 'm2'; > +include/wait_for_slave_to_start.inc > +set default_master_connection = ''; > +include/wait_for_slave_to_start.inc > +use a; > +show tables; > +Tables_in_a > +t2 > +use b; > +show tables; > +Tables_in_b > +t2 > +use c; > +show tables; > +Tables_in_c > +t2 > +#CleanUp > +drop database a; > +drop database b; > +drop database c; > +stop all slaves; > +Warnings: > +Note 1938 SLAVE 'm2' stopped > +Note 1938 SLAVE '' stopped > +Note 1938 SLAVE 'm1' stopped > +SET default_master_connection = "m1"; > +include/wait_for_slave_to_stop.inc > +SET default_master_connection = "m2"; > +include/wait_for_slave_to_stop.inc > +SET default_master_connection = ""; > +include/wait_for_slave_to_stop.inc To the test part, I suggest we always follow a well-know description pattern (which is a great time saver when it comes to future test maintenance, apart from the current reviewing), to include the task id (present in the name though) and title, what failed and *how* the test proves the fixes. I can bet the failure was also exposed by reset @@global.'connection_name'.'rule_type' variables, which the test is really missing. Could you please add up that? > diff --git a/mysql-test/suite/multi_source/mdev-8874.test b/mysql-test/suite/multi_source/mdev-8874.test > new file mode 100644 > index 00000000000..b6d1aafe139 > --- /dev/null > +++ b/mysql-test/suite/multi_source/mdev-8874.test > @@ -0,0 +1,123 @@ > +--source include/not_embedded.inc > +--source include/have_innodb.inc > +--source include/have_debug.inc > + > +--connect (server_1,127.0.0.1,root,,,$SERVER_MYPORT_1) > +--connect (server_2,127.0.0.1,root,,,$SERVER_MYPORT_2) > +--connect (server_3,127.0.0.1,root,,,$SERVER_MYPORT_3) > +--connect (server_4,127.0.0.1,root,,,$SERVER_MYPORT_4) > + > +--connection server_1 > +create database a; > +use a; > +create table t1(a int); > +insert into t1 values(1); > +create table t2(a int); > +insert into t2 values(1); > +--save_master_pos > + > +--connection server_2 > +create database b; > +use b; > +create table t1(a int); > +insert into t1 values(1); > +create table t2(a int); > +insert into t2 values(1); > +--save_master_pos > + > +--connection server_3 > +create database c; > +use c; > +create table t1(a int); > +insert into t1 values(1); > +create table t2(a int); > +insert into t2 values(1); > +--save_master_pos > + > +--connection server_4 > +--disable_warnings > +--replace_result $SERVER_MYPORT_1 MYPORT_1 > +eval change master 'm1' to master_port=$SERVER_MYPORT_1 , master_host='127.0.0.1', master_user='root'; > +--replace_result $SERVER_MYPORT_2 MYPORT_2 > +eval change master 'm2' to master_port=$SERVER_MYPORT_2 , master_host='127.0.0.1', master_user='root'; > +--replace_result $SERVER_MYPORT_3 MYPORT_3 > +eval change master to master_port=$SERVER_MYPORT_3 , master_host='127.0.0.1', master_user='root'; > +start all slaves; > +set default_master_connection = 'm1'; > +--source include/wait_for_slave_to_start.inc > +set default_master_connection = 'm2'; > +--source include/wait_for_slave_to_start.inc > +set default_master_connection = ''; > +--source include/wait_for_slave_to_start.inc > + > +--enable_warnings > +--sync_with_master 0,'m1' > +--sync_with_master 0,'m2' > +--sync_with_master 0,'' > +use a; When the test proves something via printing out, it's always good to prepare the output with an --echo what-is-expected so a thankful reader will bless you .. and the others may mercy :-) > +show tables; > +use b; > +show tables; > +use c; > +show tables; > +--echo #TEST > +STOP ALL SLAVES; > +RESET SLAVE 'm1' ALL ; > +RESET SLAVE 'm2' ALL ; > +RESET SLAVE ALL ; > +drop database a; > +drop database b; > +drop database c; > +--replace_result $SERVER_MYPORT_1 MYPORT_1 > +eval change master 'm1' to master_port=$SERVER_MYPORT_1 , master_host='127.0.0.1', master_user='root'; > +--replace_result $SERVER_MYPORT_2 MYPORT_2 > +eval change master 'm2' to master_port=$SERVER_MYPORT_2 , master_host='127.0.0.1', master_user='root'; > +--replace_result $SERVER_MYPORT_3 MYPORT_3 > +eval change master to master_port=$SERVER_MYPORT_3 , master_host='127.0.0.1', master_user='root'; > +start all slaves; > +set default_master_connection = 'm1'; > +--source include/wait_for_slave_to_start.inc > +set default_master_connection = 'm2'; > +--source include/wait_for_slave_to_start.inc > +set default_master_connection = ''; > +--source include/wait_for_slave_to_start.inc > +--sync_with_master 0,'m1' > +--sync_with_master 0,'m2' > +--sync_with_master 0,'' > + Naturally, the same applies to the following block. > +use a; > +show tables; > +use b; > +show tables; > +use c; > +show tables; > + > + > +#--echo #restart the server > +#--source include/restart_mysqld.inc > + > + > +--echo #CleanUp > +--connection server_1 > +drop database a; > +--save_master_pos > + > +--connection server_2 > +drop database b; > +--save_master_pos > + > +--connection server_3 > +drop database c; > +--save_master_pos > + > +--connection server_4 > +--sync_with_master 0,'m1' > +--sync_with_master 0,'m2' > +--sync_with_master 0,'' > +stop all slaves; > +SET default_master_connection = "m1"; > +--source include/wait_for_slave_to_stop.inc > +SET default_master_connection = "m2"; > +--source include/wait_for_slave_to_stop.inc > +SET default_master_connection = ""; > +--source include/wait_for_slave_to_stop.inc > diff --git a/sql/rpl_mi.cc b/sql/rpl_mi.cc > index 70e60b1d4ad..7aea89337a7 100644 > --- a/sql/rpl_mi.cc > +++ b/sql/rpl_mi.cc > @@ -122,8 +122,6 @@ Master_info::~Master_info() > */ > if (strncmp(connection_name.str, STRING_WITH_LEN("wsrep"))) > #endif > - rpl_filters.delete_element(connection_name.str, connection_name.length, > - (void (*)(const char*, uchar*)) free_rpl_filter); > my_free(connection_name.str); > delete_dynamic(&ignore_server_ids); > mysql_mutex_destroy(&run_lock); > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits [View Less]

2 1

[Maria-developers] GSoC [MDEV-6017]
by Alexey Mogilyovkin 10 Jun '19

10 Jun '19

Hello. This is the thread where I would like to publish my weekly reports every Monday. During preparatory weeks I set up development environment. I briefly examined testing framework, debugging process and now I am ready to start work. Work progress will be available on my github repository in branch "MDEV-6017" https://github.com/ZeroICQ/server/tree/MDEV-6017

3 5

[Maria-developers] Weekly Report
by Rucha Deodhar 10 Jun '19

10 Jun '19

Report for week 2: Hello! This week I ran the test case for parser written in last week and made changes in the parser according to the suggestions given, tried making insert...select..returning work. I analysed how RETURNING in delete is implemented, how insert is implemented, and how the data can be sent, which functions are responsible and what those functions do, to carry out various steps and what these steps are in order to send the data to the client. I came up with a plan where code … [View More]

1 0

[Maria-developers] MDEV-17951 mariadb symlinks
by Rasmus Johansson 09 Jun '19

09 Jun '19

Hi Serg, I've addressed general review requests by Wlad and review requests by Otto related to Debian packaging. The latest commit is here: https://github.com/MariaDB/server/commit/030c299f76fac044d43325abb02b5751fe… Could you give it final review to see if it can be pushed to main 10.4? Rasmus -- Rasmus Johansson, VP Engineering MariaDB | t: +358 50 499 9589 | Skype: ratzpo

1 0