- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] 53d0e921794: MDEV-11094: Blackhole table updates on slave fail when row annotation is enabled
by andrei.elkin＠pp.inet.fi 22 May '19

22 May '19

Sujatha, salute! Thanks for a detailed description! I would add up to it a minor piece > revision-id: 53d0e9217943719b806ef9fa1cac0a699df4839c (mariadb-10.1.39-31-g53d0e921794) > parent(s): 47637a3dd13d19e897a7cbfd1499f1bf3b2fdb2a > author: Sujatha > committer: Sujatha > timestamp: 2019-05-17 13:11:49 +0530 > message: > > MDEV-11094: Blackhole table updates on slave fail when row annotation is enabled > > Problem: > ======= > rpl_blackhole.test fails when executed with following options > mysqld=--binlog_annotate_row_events=1, mysqld=--replicate_annotate_row_events=1 > > Test output: > ------------ > worker[1] Using MTR_BUILD_THREAD 300, with reserved ports 16000..16019 > rpl.rpl_blackhole_bug 'mix' [ pass ] 791 > rpl.rpl_blackhole_bug 'row' [ fail ] > Replicate_Wild_Ignore_Table > Last_Errno 1032 > Last_Error Could not execute Update_rows_v1 event on table test.t1; Can't find > record in 't1', Error_code: 1032; handler error HA_ERR_END_OF_FILE; the event's > master log master-bin.000001, end_log_pos 1510 > > Analysis: > ========= > Enabling "replicate_annotate_row_events" on slave, Tells the slave to write > annotate rows events received from the master to its own binary log. The > received annotate events are applied after the Gtid event as shown below. > thd->query() will be set to the actual query received from the master, through > annotate event. Annotate_rows event should not be deleted after the event is > applied as the thd->query will be used to generate new Annotate_rows event > during applying the subsequent Rows events. [ here ] The Table_map and Rows_log_event that follow do not touch thd->query(). Notice that the being suggested sentence is sufficient to explain the not-NULL without the sentence below (fill free to remove then): > After the last Rows event has been > applied, the saved Annotate_rows event (if any) will be deleted. > > master-bin.000001 | 457 | Gtid | BEGIN GTID 0-1-2 > master-bin.000001 | 495 | Annotate_rows | insert into t values (10) > master-bin.000001 | 539 | Table_map | table_id: 19 (test.t) > master-bin.000001 | 579 | Write_rows_v1 | table_id: 19 flags: STMT_END_F > master-bin.000001 | 613 | Xid | COMMIT /* xid=7 */ > > In balckhole engine all the DML operations are noops as they donot store any > data. They simply return success without doing any operation. But the existing > strictly expects thd->query() to be 'NULL' to identify that row based > replication is in use. This assumption will fail when row annotations are > enabled as the query is not 'NULL'. Hence various row based operations like > 'update', 'delete', 'index lookup' will fail when row annotations are enabled. > > Fix: > === > Extend the row based replication check to include row annotations as well. > i.e Either the thd->query() is NULL or thd->query() points to query and row > annotations are in use. This works, but the check does not really refers to the Annotate. So in principle thd->query() may turn in not-NULL of different reasons. Besides the check is multiplied over few places (again hopefully covering all). We can do it simpler actually: Rows_log_event::do_apply_event() { ... do { /* in_use can have been set to NULL in close_tables_for_reopen */ THD* old_thd= table->in_use; if (!table->in_use) table->in_use= thd; --> add a check whether Annotate has been done and when so store/restore --> thd->query() like `old_thd` above/below, and pass it as NULL (thd->reset_query()) to the following: error= do_exec_row(rgi); if (error) DBUG_PRINT("info", ("error: %s", HA_ERR(error))); DBUG_ASSERT(error != HA_ERR_RECORD_DELETED); table->in_use = old_thd; I also suggest to check out mysqlbinlog run. There seems to be no test for that though, so one needs to be written. The goal would be to prove a created binlog output is as expected (e.g to include Annotate). Cheers, Andrei > > --- > mysql-test/suite/rpl/t/rpl_blackhole-master.opt | 1 + > mysql-test/suite/rpl/t/rpl_blackhole-slave.opt | 1 + > storage/blackhole/ha_blackhole.cc | 28 +++++++++++++++++++------ > 3 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/mysql-test/suite/rpl/t/rpl_blackhole-master.opt b/mysql-test/suite/rpl/t/rpl_blackhole-master.opt > new file mode 100644 > index 00000000000..1ad0b884c60 > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_blackhole-master.opt > @@ -0,0 +1 @@ > +--binlog_annotate_row_events > diff --git a/mysql-test/suite/rpl/t/rpl_blackhole-slave.opt b/mysql-test/suite/rpl/t/rpl_blackhole-slave.opt > new file mode 100644 > index 00000000000..7ac6a84faa7 > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_blackhole-slave.opt > @@ -0,0 +1 @@ > +--binlog_annotate_row_events --replicate_annotate_row_events > diff --git a/storage/blackhole/ha_blackhole.cc b/storage/blackhole/ha_blackhole.cc > index 01aaa9ea15f..43bcdc541a1 100644 > --- a/storage/blackhole/ha_blackhole.cc > +++ b/storage/blackhole/ha_blackhole.cc > @@ -25,6 +25,16 @@ > #include "ha_blackhole.h" > #include "sql_class.h" // THD, SYSTEM_THREAD_SLAVE_SQL > > +static bool is_row_based_replication(THD *thd) > +{ > + /* > + A row event which has its thd->query() == NULL or a row event which has > + replicate_annotate_row_events enabled. In the later case the thd->query() > + will be pointing to the query, received through replicated annotate event > + from master. > + */ > + return ((thd->query() == NULL) || thd->variables.binlog_annotate_row_events); > +} > /* Static declarations for handlerton */ > > static handler *blackhole_create_handler(handlerton *hton, > @@ -109,7 +119,8 @@ int ha_blackhole::update_row(const uchar *old_data, uchar *new_data) > { > DBUG_ENTER("ha_blackhole::update_row"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > DBUG_RETURN(0); > DBUG_RETURN(HA_ERR_WRONG_COMMAND); > } > @@ -118,7 +129,8 @@ int ha_blackhole::delete_row(const uchar *buf) > { > DBUG_ENTER("ha_blackhole::delete_row"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > DBUG_RETURN(0); > DBUG_RETURN(HA_ERR_WRONG_COMMAND); > } > @@ -135,7 +147,8 @@ int ha_blackhole::rnd_next(uchar *buf) > int rc; > DBUG_ENTER("ha_blackhole::rnd_next"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > rc= 0; > else > rc= HA_ERR_END_OF_FILE; > @@ -220,7 +233,8 @@ int ha_blackhole::index_read_map(uchar * buf, const uchar * key, > int rc; > DBUG_ENTER("ha_blackhole::index_read"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > rc= 0; > else > rc= HA_ERR_END_OF_FILE; > @@ -235,7 +249,8 @@ int ha_blackhole::index_read_idx_map(uchar * buf, uint idx, const uchar * key, > int rc; > DBUG_ENTER("ha_blackhole::index_read_idx"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > rc= 0; > else > rc= HA_ERR_END_OF_FILE; > @@ -249,7 +264,8 @@ int ha_blackhole::index_read_last_map(uchar * buf, const uchar * key, > int rc; > DBUG_ENTER("ha_blackhole::index_read_last"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > rc= 0; > else > rc= HA_ERR_END_OF_FILE; > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

Re: [Maria-developers] 9e176e81b72: MDEV-18531 : Use WolfSSL instead of YaSSL as "bundled" SSL/encryption library
by Sergei Golubchik 21 May '19

21 May '19

Hi, Vladislav! It's good to see how small the patch is, and that wolfssl is more OpenSSL compatible than YaSSL. Few comments below. On May 19, Vladislav Vaintroub wrote: > revision-id: 9e176e81b72 (mariadb-10.4.4-61-g9e176e81b72) > parent(s): ea679c88c32 > author: Vladislav Vaintroub <wlad(a)mariadb.com> > committer: Vladislav Vaintroub <wlad(a)mariadb.com> > timestamp: 2019-05-01 11:28:56 +0100 > message: > > MDEV-18531 : Use WolfSSL instead of YaSSL as "bundled" SSL/encryption library > > - Add new submodule for WolfSSL > - Build and use wolfssl and wolfcrypt instead of yassl/taocrypt > - Use HAVE_WOLFSSL instead of HAVE_YASSL > - Increase MY_AES_CTX_SIZE, to avoid compile time asserts in my_crypt.cc > (sizeof(EVP_CIPHER_CTX) is larger on WolfSSL) > > diff --git a/cmake/ssl.cmake b/cmake/ssl.cmake > index aa42333a5c8..79a531ec323 100644 > --- a/cmake/ssl.cmake > +++ b/cmake/ssl.cmake > @@ -48,29 +48,19 @@ MACRO (CHANGE_SSL_SETTINGS string) > ENDMACRO() > > MACRO (MYSQL_USE_BUNDLED_SSL) > - SET(INC_DIRS > - ${CMAKE_SOURCE_DIR}/extra/yassl/include > - ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/include > + SET(INC_DIRS > + ${CMAKE_SOURCE_DIR}/extra/wolfssl/wolfssl > + ${CMAKE_SOURCE_DIR}/extra/wolfssl/wolfssl/wolfssl > + #${CMAKE_SOURCE_DIR}/extra/wolfssl/taocrypt/include don't you want to remove these (and many below) commented lines? > ) > - SET(SSL_LIBRARIES yassl taocrypt) > + SET(SSL_LIBRARIES wolfssl wolfcrypt) > SET(SSL_INCLUDE_DIRS ${INC_DIRS}) > - SET(SSL_INTERNAL_INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/mySTL) > + SET(SSL_DEFINES "-DHAVE_OPENSSL -DHAVE_WOLFSSL -DOPENSSL_ALL -DWOLFSSL_MYSQL_COMPATIBLE -DWC_NO_HARDEN") > - SET(SSL_DEFINES "-DHAVE_YASSL -DYASSL_PREFIX -DHAVE_OPENSSL -DMULTI_THREADED") > - SET(HAVE_ERR_remove_thread_state OFF CACHE INTERNAL "yassl doesn't have ERR_remove_thread_state") > - SET(HAVE_EncryptAes128Ctr OFF CACHE INTERNAL "yassl doesn't support AES-CTR") > - SET(HAVE_EncryptAes128Gcm OFF CACHE INTERNAL "yassl doesn't support AES-GCM") > + SET(HAVE_ERR_remove_thread_state ON CACHE INTERNAL "wolfssl doesn't have ERR_remove_thread_state") > + SET(HAVE_EncryptAes128Ctr ON CACHE INTERNAL "wolfssl does support AES-CTR") > + SET(HAVE_EncryptAes128Gcm OFF CACHE INTERNAL "wolfssl does not support AES-GCM") > CHANGE_SSL_SETTINGS("bundled") > - ADD_SUBDIRECTORY(extra/yassl) > - ADD_SUBDIRECTORY(extra/yassl/taocrypt) > - GET_TARGET_PROPERTY(src yassl SOURCES) > - FOREACH(file ${src}) > - SET(SSL_SOURCES ${SSL_SOURCES} ${CMAKE_SOURCE_DIR}/extra/yassl/${file}) > - ENDFOREACH() > - GET_TARGET_PROPERTY(src taocrypt SOURCES) > - FOREACH(file ${src}) > - SET(SSL_SOURCES ${SSL_SOURCES} > - ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/${file}) > - ENDFOREACH() > + ADD_SUBDIRECTORY(extra/wolfssl) > MESSAGE_ONCE(SSL_LIBRARIES "SSL_LIBRARIES = ${SSL_LIBRARIES}") > ENDMACRO() > > diff --git a/extra/wolfssl/CMakeLists.txt b/extra/wolfssl/CMakeLists.txt > new file mode 100644 > index 00000000000..c1165ad6c72 > --- /dev/null > +++ b/extra/wolfssl/CMakeLists.txt > @@ -0,0 +1,119 @@ > +# CMakeLists.txt > +# > +# Copyright (C) 2006-2015 wolfSSL Inc. > +# > +# This file is part of wolfSSL. (formerly known as CyaSSL) > +# > +# wolfSSL is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# wolfSSL is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write to the Free Software > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA > + > +SET(WOLFSSL_SRCDIR ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl/src) > +ADD_DEFINITIONS(${SSL_DEFINES}) > +ADD_DEFINITIONS( > + -DWOLFSSL_MYSQL_COMPATIBLE > + -DHAVE_ECC > + -DECC_TIMING_RESISTANT > + -DBUILDING_WOLFSSL > + -DHAVE_HASHDRBG > + -DWOLFSSL_AES_DIRECT > + -DWOLFSSL_SHA384 > + -DWOLFSSL_SHA512 > + -DWOLFSSL_SHA224 > + -DSESSION_CERT > + -DKEEP_OUR_CERT > + -DWOLFSSL_STATIC_RSA > + -DWC_RSA_BLINDING > + -DHAVE_TLS_EXTENSIONS > + -DHAVE_AES_ECB > + -DWOLFSSL_AES_COUNTER > + -DNO_WOLFSSL_STUB) > + > +SET(WOLFSSL_SOURCES > + ${WOLFSSL_SRCDIR}/crl.c > + ${WOLFSSL_SRCDIR}/internal.c > + ${WOLFSSL_SRCDIR}/keys.c > + ${WOLFSSL_SRCDIR}/tls.c > + ${WOLFSSL_SRCDIR}/wolfio.c > + ${WOLFSSL_SRCDIR}/ocsp.c > + ${WOLFSSL_SRCDIR}/ssl.c) > +ADD_DEFINITIONS(-DWOLFSSL_LIB) > +INCLUDE_DIRECTORIES(BEFORE ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl) > +IF(MSVC) > + # size_t to long truncation warning > + ADD_DEFINITIONS(-wd4267) > +ENDIF() > + > + > +ADD_CONVENIENCE_LIBRARY(wolfssl ${WOLFSSL_SOURCES}) > + > +# Workaround linker crash with older Ubuntu binutils > +# e.g aborting at ../../bfd/merge.c line 873 in _bfd_merged_section_offset > +IF((CMAKE_SYSTEM_NAME MATCHES "Linux") AND (CMAKE_LINKER STREQUAL "/usr/bin/ld")) > + EXECUTE_PROCESS(COMMAND ${CMAKE_LINKER} -v OUTPUT_VARIABLE LINKER_VERSION_STR) > + STRING(REGEX MATCH "[0-9]+\\.[0-9]+" LINKER_VERSION > + "${LINKER_VERSION_STR}") > + IF(LINKER_VERSION AND (LINKER_VERSION VERSION_LESS 2.25)) > + STRING(REPLACE "-g " "-g1 " CMAKE_C_FLAGS_RELWITHDEBINFO > + ${CMAKE_C_FLAGS_RELWITHDEBINFO}) > + STRING(REPLACE "-g " "-g1 " CMAKE_C_FLAGS_DEBUG > + ${CMAKE_C_FLAGS_DEBUG}) > + STRING(REPLACE "-ggdb3 " " " CMAKE_C_FLAGS_RELWITHDEBINFO > + ${CMAKE_C_FLAGS_RELWITHDEBINFO}) > + STRING(REPLACE "-ggdb3 " " " CMAKE_C_FLAGS_DEBUG > + ${CMAKE_C_FLAGS_DEBUG}) > + ENDIF() > +ENDIF() I would've just disabled -g\w+ unconditionally, for a simpler CMakeLists.txt We aren't going to debug wolfssl anyway (and if I'd need to debug it, for some weird reason, I know how to enable debugging temporarily) > + > + > +SET(WOLFCRYPT_SRCDIR ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl/wolfcrypt/src) > +SET(WOLFCRYPT_SOURCES > +${WOLFCRYPT_SRCDIR}/aes.c > +${WOLFCRYPT_SRCDIR}/arc4.c > +${WOLFCRYPT_SRCDIR}/asn.c > +#${WOLFCRYPT_SRCDIR}/blake2b.c > +#${WOLFCRYPT_SRCDIR}/camellia.c > +#${WOLFCRYPT_SRCDIR}/chacha.c > +${WOLFCRYPT_SRCDIR}/coding.c > +#${WOLFCRYPT_SRCDIR}/compress.c > +${WOLFCRYPT_SRCDIR}/des3.c > +${WOLFCRYPT_SRCDIR}/dh.c > +${WOLFCRYPT_SRCDIR}/dsa.c > +${WOLFCRYPT_SRCDIR}/ecc.c > +${WOLFCRYPT_SRCDIR}/error.c > +#${WOLFCRYPT_SRCDIR}/hc128.c > +${WOLFCRYPT_SRCDIR}/hmac.c > +${WOLFCRYPT_SRCDIR}/integer.c > +${WOLFCRYPT_SRCDIR}/logging.c > +#${WOLFCRYPT_SRCDIR}/md2.c > +${WOLFCRYPT_SRCDIR}/md4.c > +${WOLFCRYPT_SRCDIR}/md5.c > +${WOLFCRYPT_SRCDIR}/memory.c > +#${WOLFCRYPT_SRCDIR}/pkcs7.c > +${WOLFCRYPT_SRCDIR}/pkcs12.c > +#${WOLFCRYPT_SRCDIR}/poly1305.c > +${WOLFCRYPT_SRCDIR}/pwdbased.c > +${WOLFCRYPT_SRCDIR}/rabbit.c > +${WOLFCRYPT_SRCDIR}/random.c > +#${WOLFCRYPT_SRCDIR}/ripemd.c > +${WOLFCRYPT_SRCDIR}/rsa.c > +${WOLFCRYPT_SRCDIR}/sha.c > +${WOLFCRYPT_SRCDIR}/sha256.c > +${WOLFCRYPT_SRCDIR}/sha512.c > +#${WOLFCRYPT_SRCDIR}/tfm.c > +${WOLFCRYPT_SRCDIR}/wc_port.c > +${WOLFCRYPT_SRCDIR}/wc_encrypt.c > +${WOLFCRYPT_SRCDIR}/hash.c > +${WOLFCRYPT_SRCDIR}/wolfmath.c) > +ADD_CONVENIENCE_LIBRARY(wolfcrypt ${WOLFCRYPT_SOURCES}) > + > diff --git a/extra/wolfssl/wolfssl b/extra/wolfssl/wolfssl > new file mode 160000 > index 00000000000..dc313ccf6ee > --- /dev/null > +++ b/extra/wolfssl/wolfssl > @@ -0,0 +1 @@ > +Subproject commit dc313ccf6ee0e99fb5a13793b82e412f396e11c6 I think we should only use wolfssl (any external project, actually) only at release points, not at intermediate commits. 4.0.0-stable should be ok, I guess. And, generaly, pefer to use system globally installed library, if available. Although in case of wolfssl (and libmarias3) I would not bother. > diff --git a/include/ssl_compat.h b/include/ssl_compat.h > index c94b9671d5f..105405efff2 100644 > --- a/include/ssl_compat.h > +++ b/include/ssl_compat.h > @@ -17,9 +17,9 @@ > #include <openssl/opensslv.h> > > /* OpenSSL version specific definitions */ > -#if !defined(HAVE_YASSL) && defined(OPENSSL_VERSION_NUMBER) > +#if defined(OPENSSL_VERSION_NUMBER) > > -#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER) > +#if (OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER)) || defined (HAVE_WOLFSSL) What's OPENSSL_VERSION_NUMBER in wolfssl? > #define HAVE_X509_check_host 1 May be move it to ssl.cmake? Where other such defines are. > #endif > > diff --git a/include/sslopt-case.h b/include/sslopt-case.h > index 4a8c65948cb..b129a97cc76 100644 > --- a/include/sslopt-case.h > +++ b/include/sslopt-case.h > @@ -29,8 +29,8 @@ > One can disable SSL later by using --skip-ssl or --ssl=0 > */ > opt_use_ssl= 1; > - /* crl has no effect in yaSSL */ > -#ifdef HAVE_YASSL > +#ifdef HAVE_WOLFSSL > + /* CRL does not work with WolfSSL */ Still? > opt_ssl_crl= NULL; > opt_ssl_crlpath= NULL; > #endif > diff --git a/mysys_ssl/my_crypt.cc b/mysys_ssl/my_crypt.cc > index 2d6f5188034..1ce0c6bbc03 100644 > --- a/mysys_ssl/my_crypt.cc > +++ b/mysys_ssl/my_crypt.cc > @@ -18,14 +18,10 @@ > #include <my_global.h> > #include <string.h> > > -#ifdef HAVE_YASSL > -#include "yassl.cc" delete yassl.cc file, perhaps? > -#else > #include <openssl/evp.h> > #include <openssl/aes.h> > #include <openssl/err.h> > #include <openssl/rand.h> > -#endif > > #include <my_crypt.h> > #include <ssl_compat.h> > @@ -70,8 +66,24 @@ class MyCTX > } > virtual int finish(uchar *dst, uint *dlen) > { > +#ifdef HAVE_WOLFSSL > + /* > + Bug in WolfSSL - sometimes EVP_CipherFinal_ex > + returns success without setting destination length > + when it should return error. > + We catch it by presetting invalid value for length, > + and checking if it has changed after the call. > + > + See https://github.com/wolfSSL/wolfssl/issues/2092 > + */ > + *dlen= UINT_MAX; > +#endif I suppose you can remove it. This is fixed as of 4.0.0-stable tag. > if (!EVP_CipherFinal_ex(ctx, dst, (int*)dlen)) > return MY_AES_BAD_DATA; > +#ifdef HAVE_WOLFSSL > + if (*dlen == UINT_MAX) > + return MY_AES_BAD_DATA; > +#endif > return MY_AES_OK; > } > }; > diff --git a/plugin/file_key_management/parser.cc b/plugin/file_key_management/parser.cc > index 13a9dfa0cb6..27277f62994 100644 > --- a/plugin/file_key_management/parser.cc > +++ b/plugin/file_key_management/parser.cc > @@ -103,7 +103,6 @@ openssl enc -aes-256-cbc -md sha1 -k "secret" -in keys.txt -out keys.enc > EVP_BytesToKey(EVP_aes_256_cbc(), EVP_sha1(), salt, > secret, strlen(secret), 1, key, iv); > > - but alas! we want to support yassl too does wolfssl have an equivalent of EVP_BytesToKey? if not, the comment still holds. > */ > > void Parser::bytes_to_key(const unsigned char *salt, const char *input, > diff --git a/sql/mysqld.cc b/sql/mysqld.cc > index bfa03aa57c1..8c3a59c55d1 100644 > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -7317,13 +7315,13 @@ static int show_ssl_get_verify_mode(THD *thd, SHOW_VAR *var, char *buff, > { > var->type= SHOW_LONG; > var->value= buff; > -#ifndef HAVE_YASSL > +#ifndef HAVE_WOLFSSL > if( thd->net.vio && thd->net.vio->ssl_arg ) > *((long *)buff)= (long)SSL_get_verify_mode((SSL*)thd->net.vio->ssl_arg); no SSL_get_verify_mode or an analog in wolfssl? I'm asking, as I see it has gone a long way towards OpenSSL compatibility > else > *((long *)buff)= 0; > #else > - *((long *)buff) = 0; > + *((long *)buff)= 0; > #endif > return 0; > } Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

Re: [Maria-developers] ddec45aa50e: MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER trying to add bad CHECK
by Sergei Golubchik 20 May '19

20 May '19

Hi, Oleksandr! On May 20, Oleksandr Byelkin wrote: > Alter table will trigger recompilation of the SP (as I remember all SPs) I've run this test, it complains about duplicate constraint name. > > What about this: > > > > CREATE TABLE t1 (a INT); > > CREATE PROCEDURE sp() ALTER TABLE t1 ADD CONSTRAINT CHECK (b > 0); > > --error ER_BAD_FIELD_ERROR > > CALL sp; > > alter table t1 add column b int, add constraint check (b < 10); > > CALL sp; > > > > ? > > second CALL will not regenerate the name of the constraint. > > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] ddec45aa50e: MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER trying to add bad CHECK
by Sergei Golubchik 20 May '19

20 May '19

Hi, Oleksandr! On May 20, Oleksandr Byelkin wrote: > On Mon, May 20, 2019 at 3:11 PM Sergei Golubchik <serg(a)mariadb.org> wrote: > > On May 20, Oleksandr Byelkin wrote: > > > revision-id: ddec45aa50e (mariadb-10.2.24-11-gddec45aa50e) > > > parent(s): 50999738eae > > > author: Oleksandr Byelkin <sanja(a)mariadb.com> > > > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > > > timestamp: 2019-05-14 14:01:15 +0200 > > > message: > > > > > > MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / > > > my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER > > > trying to add bad CHECK > > > > > > In case of error the SP can be executed without re-comilation and so > > > will reuse constructed constaint name, so the name should be allocated > > > in the statement memory. > > > > Is that right? You're saying that the "unique constraint name" will be > > generated once and reused? > > > > But on the second SP invocation is might no longer be unique. I'd think > > that `ALTER TABLE t1 ADD CONSTRAINT CHECK (x > 0)` should add a new > > constraint, not "add a new contraint with the name that used to be > > unique when the SP was invoked for the first time". > > With the second call we have 2 possibility: > > 1) first execution failed and so we can reuse the name > 2) first execution succeed and we will re-compile SP or re-prepare PS and > so will generate new name (that is why we mostly had no problems) What about this: CREATE TABLE t1 (a INT); CREATE PROCEDURE sp() ALTER TABLE t1 ADD CONSTRAINT CHECK (b > 0); --error ER_BAD_FIELD_ERROR CALL sp; alter table t1 add column b int, add constraint check (b < 10); CALL sp; ? second CALL will not regenerate the name of the constraint. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] ddec45aa50e: MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER trying to add bad CHECK
by Sergei Golubchik 20 May '19

20 May '19

Hi, Oleksandr! On May 20, Oleksandr Byelkin wrote: > revision-id: ddec45aa50e (mariadb-10.2.24-11-gddec45aa50e) > parent(s): 50999738eae > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2019-05-14 14:01:15 +0200 > message: > > MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / > my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER > trying to add bad CHECK > > In case of error the SP can be executed without re-comilation and so > will reuse constructed constaint name, so the name should be allocated > in the statement memory. Is that right? You're saying that the "unique constraint name" will be generated once and reused? But on the second SP invocation is might no longer be unique. I'd think that `ALTER TABLE t1 ADD CONSTRAINT CHECK (x > 0)` should add a new constraint, not "add a new contraint with the name that used to be unique when the SP was invoked for the first time". > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index 3df877792cc..ecd5de3a09d 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -4185,9 +4185,15 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, > while ((check= c_it++)) > { > if (!check->name.length) > + { > + Query_arena backup; > + Query_arena *arena= thd->activate_stmt_arena_if_needed(&backup); > make_unique_constraint_name(thd, &check->name, > &alter_info->check_constraint_list, > &nr); > + if (arena) > + thd->restore_active_arena(arena, &backup); > + } > { > /* Check that there's no repeating constraint names. */ > List_iterator_fast<Virtual_column_info> Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 66349551bf3: MDEV-16214: Incorrect plan taken by the optimizer , uses INDEX instead of ref access with ORDER BY
by Sergey Petrunia 19 May '19

19 May '19

Hi Varun, The patch is mostly fine. Please find some input on the comments / coding style below. Ok to push after it is addressed. On Fri, May 17, 2019 at 01:44:36PM +0530, Varun wrote: > revision-id: 66349551bf3a266fbefaebfe044abd05108723c2 (mariadb-10.3.12-205-g66349551bf3) > parent(s): 3d56adbfac394b2b3ffd22a89fe7c2978ed9a505 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2019-05-17 13:44:05 +0530 > message: > > MDEV-16214: Incorrect plan taken by the optimizer , uses INDEX instead of ref access with ORDER BY > > The issue in this case is that we take in account the estimates from quick keys instead of rec_per_key. > The estimates for quick keys are better than rec_per_key only if we have ref(const), so we need to check > that all keyparts in the ref key are of the type ref(const). > > --- > mysql-test/main/order_by.result | 57 +++++++++++++++++++++++++++++++++++++++++ > mysql-test/main/order_by.test | 37 ++++++++++++++++++++++++++ > sql/sql_select.cc | 42 ++++++++++++++++++++---------- > 3 files changed, 122 insertions(+), 14 deletions(-) ... > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index f36a68bc7ae..8cdcf3afc8b 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -9986,31 +9986,35 @@ static bool create_ref_for_key(JOIN *join, JOIN_TAB *j, > j->ref.null_rejecting|= (key_part_map)1 << i; > keyuse_uses_no_tables= keyuse_uses_no_tables && !keyuse->used_tables; > /* > - Todo: we should remove this check for thd->lex->describe on the next > - line. With SHOW EXPLAIN code, EXPLAIN printout code no longer depends > - on it. However, removing the check caused change in lots of query > - plans! Does the optimizer depend on the contents of > - table_ref->key_copy ? If yes, do we produce incorrect EXPLAINs? > + We don't want to compute heavy expressions in EXPLAIN, an example would > + select * from t1 where t1.key=(select thats very heavy); > + > + (select thats very heavy) => is a constant here > + eg: (select avg(order_cost) from orders) => constant but expensive > */ > if (!keyuse->val->used_tables() && !thd->lex->describe) > { // Compare against constant > - store_key_item tmp(thd, > + store_key_item tmp(thd, > keyinfo->key_part[i].field, > key_buff + maybe_null, > maybe_null ? key_buff : 0, > keyinfo->key_part[i].length, > keyuse->val, > FALSE); > - if (unlikely(thd->is_fatal_error)) > - DBUG_RETURN(TRUE); > - tmp.copy(); > + if (unlikely(thd->is_fatal_error)) > + DBUG_RETURN(TRUE); > + tmp.copy(); > j->ref.const_ref_part_map |= key_part_map(1) << i ; > } > else > - *ref_key++= get_store_key(thd, > - keyuse,join->const_table_map, > - &keyinfo->key_part[i], > - key_buff, maybe_null); > + { > + *ref_key++= get_store_key(thd, > + keyuse,join->const_table_map, > + &keyinfo->key_part[i], > + key_buff, maybe_null); > + if (!keyuse->val->used_tables()) > + j->ref.const_ref_part_map |= key_part_map(1) << i ; > + } > /* > Remember if we are going to use REF_OR_NULL > But only if field _really_ can be null i.e. we force JT_REF > @@ -25256,6 +25260,8 @@ bool JOIN_TAB::save_explain_data(Explain_table_access *eta, > { > if (!(eta->ref_list.append_str(thd->mem_root, "const"))) > return 1; > + if (thd->lex->describe) > + key_ref++; This is very cryptic. Please provide an explanation, something like: create_ref_for_key() handles keypart=const equalities as follows: - non-EXPLAIN execution will copy the "const" to lookup tuple immediately and will not add an element to ref.key_copy - EXPLAIN will put an element into ref.key_copy. Since we've just printed "const" for it, we should skip it here. > } > else > { > @@ -26921,7 +26927,15 @@ test_if_cheaper_ordering(const JOIN_TAB *tab, ORDER *order, TABLE *table, > */ > if (ref_key >= 0 && ref_key != MAX_KEY && tab->type == JT_REF) > { > - if (table->quick_keys.is_set(ref_key)) > + /* > + For all the parts of the ref key we check if all of them belong > + to the type ref(const). This is done because if all parts of the ref > + key are of type ref(const), then we are sure that the estimates > + provides by quick keys is better than that provide by rec_per_key. The above is hard to read and has typos. Here's a shorter variant: If ref access uses keypart=const for all its key parts, and quick select uses the same # of key parts, then they are equivalent. Reuse #rows estimate from quick select as it is more precise. > + */ > + if (tab->ref.const_ref_part_map == make_prev_keypart_map(tab->ref.key_parts) && > + table->quick_keys.is_set(ref_key) && > + table->quick_key_parts[ref_key] == tab->ref.key_parts) Please fix indetation; One of the lines is too long. > refkey_rows_estimate= table->quick_rows[ref_key]; > else > { BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] FYI: usage of F1-F12 keys in mysql client
by Aleksey Midenkov 17 May '19

17 May '19

By adding something like this in your .inputrc: $if MYSQL "\033OP": "use information_schema; show tables;\n" "\033OQ": "use test;\n" "\033OR": "select * from " "\033OS": "show columns from " "\033[15~": "\033[17~": "\033[18~": "\033[19~": "\033[20~": "\033[21~": "\033[23~": "\033[24~": $endif you may have F1-F12 keys utilized. Different terminals have different codes (.inputrc doesn't support capability names unfortunately), so you'll have to find out your own codes via tack utiity. With strong will and enough patience it is possible to add Ctrl+, Alt+, Shift+ and their combinations as well. This works only with MDEV-17609 patch. -- All the best, Aleksey Midenkov @midenok

1 0

Re: [Maria-developers] MDEV-19284-19285v3
by Eugene Kosov 14 May '19

14 May '19

Hi, Alexander! Patch looks good to me. 13.05.2019, 17:03, "Alexander Barkov" <bar(a)mariadb.com>: > Hi Eugene, > > Please review the next round of the patch. > > Are any changes in tests still needed? > > Thanks! -- Eugene

1 0

Re: [Maria-developers] 7198e7c71fc: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING
by Sergei Golubchik 13 May '19

13 May '19

Hi, Aleksey! On Dec 23, Aleksey Midenkov wrote: > revision-id: 7198e7c71fc (versioning-1.0.6-82-g7198e7c71fc) > parent(s): 6be155757b7 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2018-12-20 13:25:00 +0300 > message: > > MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING > > * HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine > * Versioning tests support As far as I can see, there are two approaches to fixing this bug. And if we go the way of HA_EXTRA_REMEMBER_POS/HA_EXTRA_RESTORE_POS then your fix is good. But I'd still like to understand whether write_row is supposed to disturb the scan. Why should heap_write() overwrite info->current_ptr=pos and info->current_hash_ptr? Inserting a row doesn't happen at the "current position", so I don't quite understand why it modifies current_ptr at all. There is no functionality like "continue the scan from the last inserted row", right? It just doesn't make sense to me. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

[Maria-developers] Welcome to GSoC 2019
by Vicențiu Ciorbaru 10 May '19

10 May '19

Hi! As you may have noticed a few days ago, you have received an email from Google that you have been accepted as part of Google Summer of Code. Congratulations on the effort to learn about the program, get involved and submit succesful proposals. The fun part begins now! If you have not done so already, please reach out to your mentor and seek further advice about your project. Each one of your projects can be implemented in multiple ways, some more complex than others. I strongly advise you to create a concrete plan based off of your proposal with your mentor and aim to stick to it. Your mentor can guide you to ensure you have a succesful GSoC. Remember that you are here to learn about Open Source, what it means to be part of an Open Source community, to interact with the community, all while becoming better programmers. Please take advantage of this opportunity and keep your technical discussions public, on Zulip https://mariadb.zulipchat.com, and on our developer mailing list https://launchpad.net/~maria-developers. By keeping your discussions public, you get the benefit of not only having your mentor see your questions but all of the MariaDB community. You never know when someone might lend a hand in case your mentor is not yet online. :) The mantra of our community should be that we are always welcoming to new people. Do not be afraid to ask questions. There are no bad or stupid or silly questions. The only key requirement is that you first make an effort to find the answer yourself and only afterwards seek outside help. If you can show a bit of involvement, the community will show you the same. I recommend that you agree with your mentor on some form of weekly progress reporting, via either a blog-post, an email, or something similar that can be publicly shared. Hope all of you will have a great GSoC! Vicențiu MariaDB Foundation GSoC 2019 Admin

1 0