Hi, Alexey! Thanks! Few minor comments, see below
On Nov 23, Alexey Botchkov wrote: > revision-id: afedc29f773362f342756e9cdd9bbd9ed0f1abe2 > (versioning-1.0.3-68-gafedc29f773) > parent(s): 4ec8598c1dcb63499bce998142b8e5b8b09b2d30 > author: Alexey Botchkov <holyf...@askmonty.org> > committer: Alexey Botchkov <holyf...@askmonty.org> > timestamp: 2018-06-26 14:10:46 +0400 > message: > > MDEV-14024 PCRE2. > > Related changes in the server code. > > diff --git a/cmake/pcre.cmake b/cmake/pcre.cmake > index 4c113929866..264f93a60a2 100644 > --- a/cmake/pcre.cmake > +++ b/cmake/pcre.cmake > @@ -5,24 +5,17 @@ SET(WITH_PCRE "auto" CACHE STRING > > MACRO (CHECK_PCRE) > IF(WITH_PCRE STREQUAL "system" OR WITH_PCRE STREQUAL "auto") > - CHECK_LIBRARY_EXISTS(pcre pcre_stack_guard "" HAVE_PCRE_STACK_GUARD) > + CHECK_LIBRARY_EXISTS(pcre2-8 pcre2_match "" HAVE_PCRE2) > - IF(NOT CMAKE_CROSSCOMPILING) > - SET(CMAKE_REQUIRED_LIBRARIES "pcre") > - CHECK_C_SOURCE_RUNS(" > - #include <pcre.h> > - int main() { > - return -pcre_exec(NULL, NULL, NULL, -999, -999, 0, NULL, 0) < 256; > - }" PCRE_STACK_SIZE_OK) > - SET(CMAKE_REQUIRED_LIBRARIES) > - ENDIF() > ENDIF() > - IF(NOT HAVE_PCRE_STACK_GUARD OR NOT PCRE_STACK_SIZE_OK OR > - WITH_PCRE STREQUAL "bundled") > + IF(NOT HAVE_PCRE2 OR WITH_PCRE STREQUAL "bundled") > IF (WITH_PCRE STREQUAL "system") > - MESSAGE(FATAL_ERROR "system pcre is not found or unusable") > + MESSAGE(FATAL_ERROR "system pcre2-8 library is not found or unusable") > ENDIF() > - SET(PCRE_INCLUDES ${CMAKE_BINARY_DIR}/pcre ${CMAKE_SOURCE_DIR}/pcre) > - ADD_SUBDIRECTORY(pcre) > + SET(PCRE_INCLUDES ${CMAKE_BINARY_DIR}/pcre2 ${CMAKE_SOURCE_DIR}/pcre2 > + ${CMAKE_BINARY_DIR}/pcre2/src > ${CMAKE_SOURCE_DIR}/pcre2/src) That's a bit strange. I'd expect only .../pcre2/src paths, not .../pcre2. you need pcre2.h and pcre2posix.h, they're both in src/ right? > + SET(PCRE_BUILD_TESTS OFF CACHE BOOL "Disable tests.") > + SET(PCRE2_BUILD_PCRE2GREP OFF CACHE BOOL "Disable pcre2grep") > + ADD_SUBDIRECTORY(pcre2) > ENDIF() > ENDMACRO() > > diff --git a/extra/mariabackup/CMakeLists.txt > b/extra/mariabackup/CMakeLists.txt > index 7df5fa17903..a9d7153b893 100644 > --- a/extra/mariabackup/CMakeLists.txt > +++ b/extra/mariabackup/CMakeLists.txt > @@ -37,7 +37,7 @@ INCLUDE_DIRECTORIES( > ) > > IF(NOT HAVE_SYSTEM_REGEX) > - INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/pcre) > + INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/pcre2 > ${CMAKE_SOURCE_DIR}/pcre2/src) Hmm, not INCLUDE_DIRECTORIES(${PCRE_INCLUDES}) ? pcre2.h is in the ${CMAKE_BINARY_DIR}, it's generated. > ENDIF() > > IF(WITH_WSREP) > diff --git a/mysql-test/main/func_regexp_pcre.test > b/mysql-test/main/func_regexp_pcre.test > index 21600390bb2..30969b3e9ae 100644 > --- a/mysql-test/main/func_regexp_pcre.test > +++ b/mysql-test/main/func_regexp_pcre.test > @@ -382,6 +382,7 @@ SELECT 'AB' RLIKE 'A B'; > SELECT 'AB' RLIKE 'A# this is a comment\nB'; > SET default_regex_flags=DEFAULT; > > +--error ER_REGEXP_ERROR > SELECT 'Aq' RLIKE 'A\\q'; you forgot to update func_regexp_pcre.result > SET default_regex_flags='EXTRA'; > --error ER_REGEXP_ERROR > diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc > index 76f4788c1cf..450a9c54176 100644 > --- a/sql/item_cmpfunc.cc > +++ b/sql/item_cmpfunc.cc > @@ -5485,124 +5486,44 @@ bool Regexp_processor_pcre::compile(Item *item, bool > send_error) > */ > void Regexp_processor_pcre::pcre_exec_warn(int rc) const > { > - char buf[64]; > - const char *errmsg= NULL; > + PCRE2_UCHAR8 buf[128]; > THD *thd= current_thd; > > - /* > - Make a descriptive message only for those pcre_exec() error codes > - that can actually happen in MariaDB. > - */ > - switch (rc) > + int errlen= pcre2_get_error_message(rc, buf, sizeof(buf)); > + if (errlen <= 0) > { > - case PCRE_ERROR_NULL: > - errmsg= "pcre_exec: null argument passed"; > - break; > - case PCRE_ERROR_BADOPTION: > - errmsg= "pcre_exec: bad option"; > - break; > - case PCRE_ERROR_BADMAGIC: > - errmsg= "pcre_exec: bad magic - not a compiled regex"; > - break; > - case PCRE_ERROR_UNKNOWN_OPCODE: > - errmsg= "pcre_exec: error in compiled regex"; > - break; > - case PCRE_ERROR_NOMEMORY: > - errmsg= "pcre_exec: Out of memory"; > - break; > - case PCRE_ERROR_NOSUBSTRING: > - errmsg= "pcre_exec: no substring"; > - break; > - case PCRE_ERROR_MATCHLIMIT: > - errmsg= "pcre_exec: match limit exceeded"; > - break; > - case PCRE_ERROR_CALLOUT: > - errmsg= "pcre_exec: callout error"; > - break; > - case PCRE_ERROR_BADUTF8: > - errmsg= "pcre_exec: Invalid utf8 byte sequence in the subject string"; > - break; > - case PCRE_ERROR_BADUTF8_OFFSET: > - errmsg= "pcre_exec: Started at invalid location within utf8 byte > sequence"; > - break; > - case PCRE_ERROR_PARTIAL: > - errmsg= "pcre_exec: partial match"; > - break; > - case PCRE_ERROR_INTERNAL: > - errmsg= "pcre_exec: internal error"; > - break; > - case PCRE_ERROR_BADCOUNT: > - errmsg= "pcre_exec: ovesize is negative"; > - break; > - case PCRE_ERROR_RECURSIONLIMIT: > - my_snprintf(buf, sizeof(buf), "pcre_exec: recursion limit of %ld > exceeded", > - m_pcre_extra.match_limit_recursion); > - errmsg= buf; > - break; > - case PCRE_ERROR_BADNEWLINE: > - errmsg= "pcre_exec: bad newline options"; > - break; > - case PCRE_ERROR_BADOFFSET: > - errmsg= "pcre_exec: start offset negative or greater than string length"; > - break; > - case PCRE_ERROR_SHORTUTF8: > - errmsg= "pcre_exec: ended in middle of utf8 sequence"; > - break; > - case PCRE_ERROR_JIT_STACKLIMIT: > - errmsg= "pcre_exec: insufficient stack memory for JIT compile"; > - break; > - case PCRE_ERROR_RECURSELOOP: > - errmsg= "pcre_exec: Recursion loop detected"; > - break; > - case PCRE_ERROR_BADMODE: > - errmsg= "pcre_exec: compiled pattern passed to wrong bit library > function"; > - break; > - case PCRE_ERROR_BADENDIANNESS: > - errmsg= "pcre_exec: compiled pattern passed to wrong endianness > processor"; > - break; > - case PCRE_ERROR_JIT_BADOPTION: > - errmsg= "pcre_exec: bad jit option"; > - break; > - case PCRE_ERROR_BADLENGTH: > - errmsg= "pcre_exec: negative length"; > - break; > - default: > - /* > - As other error codes should normally not happen, > - we just report the error code without textual description > - of the code. > - */ > - my_snprintf(buf, sizeof(buf), "pcre_exec: Internal error (%d)", rc); > - errmsg= buf; > + my_snprintf((char *)buf, sizeof(buf), "pcre_exec: Internal error (%d)", > rc); > } > push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, > - ER_REGEXP_ERROR, ER_THD(thd, ER_REGEXP_ERROR), errmsg); > + ER_REGEXP_ERROR, ER_THD(thd, ER_REGEXP_ERROR), buf); > } > > > /** > Call pcre_exec() and send a warning if pcre_exec() returned with an error. > */ > -int Regexp_processor_pcre::pcre_exec_with_warn(const pcre *code, > - const pcre_extra *extra, > +int Regexp_processor_pcre::pcre_exec_with_warn(const pcre2_code *code, > + pcre2_match_data *data, > const char *subject, > int length, int startoffset, > - int options, int *ovector, > - int ovecsize) > + uint options) > { > - int rc= pcre_exec(code, extra, subject, length, > - startoffset, options, ovector, ovecsize); > + int rc= pcre2_match(code, (PCRE2_SPTR8) subject, (PCRE2_SIZE) length, > + (PCRE2_SIZE) startoffset, options, data, NULL); > DBUG_EXECUTE_IF("pcre_exec_error_123", rc= -123;); > - if (unlikely(rc < PCRE_ERROR_NOMATCH)) > - pcre_exec_warn(rc); > + if (unlikely(rc < PCRE2_ERROR_NOMATCH)) > + m_SubStrVec= NULL; > + else > + m_SubStrVec= pcre2_get_ovector_pointer(data); > + The function name is still pcre_exec_with_warn, the comment still says "Call pcre_exec() and send a warning". But neither seems to be happening here. where do you do warnings now? > return rc; > } > > > bool Regexp_processor_pcre::exec(const char *str, size_t length, size_t > offset) > { > - m_pcre_exec_rc= pcre_exec_with_warn(m_pcre, &m_pcre_extra, str, > (int)length, (int)offset, 0, > - m_SubStrVec, > array_elements(m_SubStrVec)); > + m_pcre_exec_rc= pcre_exec_with_warn(m_pcre, m_pcre_match_data, > + str, (int)length, (int)offset, 0); > return false; > } > > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index ccb7d3b29ed..e8abae3487d 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -5692,19 +5692,21 @@ static const char *default_regex_flags_names[]= > "DOTALL", // (?s) . matches anything including NL > "DUPNAMES", // (?J) Allow duplicate names for subpatterns > "EXTENDED", // (?x) Ignore white space and # comments > - "EXTRA", // (?X) extra features (e.g. error on unknown escape > character) > + "EXTENDED_MORE",//(?xx) Ignore white space and # comments inside cheracter perhaps, update all flag descriptions from man pcre2syntax? Like "EXTENDED", // (?x) extended: ignore white space except in classes "EXTENDED_MORE",//(?xx) as (?x) but also ignore space and tab in classes also, I don't quite like EXTENDED_MORE name. But I don't have a good suggestion. May be just re-purpose EXTENDED to do (?xx) without introducing a new flag? > + "EXTRA", // means nothing since PCRE2 may be a warning when it's used? > "MULTILINE", // (?m) ^ and $ match newlines within data > "UNGREEDY", // (?U) Invert greediness of quantifiers > 0 > }; > static const int default_regex_flags_to_pcre[]= > { > - PCRE_DOTALL, > - PCRE_DUPNAMES, > - PCRE_EXTENDED, > - PCRE_EXTRA, > - PCRE_MULTILINE, > - PCRE_UNGREEDY, > + PCRE2_DOTALL, > + PCRE2_DUPNAMES, > + PCRE2_EXTENDED, > + PCRE2_EXTENDED_MORE, > + 0, /* EXTRA flag not available since PCRE2 */ > + PCRE2_MULTILINE, > + PCRE2_UNGREEDY, > 0 > }; > int default_regex_flags_pcre(const THD *thd) > diff --git > a/storage/tokudb/PerconaFT/cmake_modules/TokuFeatureDetection.cmake > b/storage/tokudb/PerconaFT/cmake_modules/TokuFeatureDetection.cmake > index 2f04a33558a..51257febc0d 100644 > --- a/storage/tokudb/PerconaFT/cmake_modules/TokuFeatureDetection.cmake > +++ b/storage/tokudb/PerconaFT/cmake_modules/TokuFeatureDetection.cmake > @@ -1,6 +1,8 @@ > ## feature detection > find_package(Threads) > -find_package(ZLIB REQUIRED) > +IF(WITH_EXTERNAL_ZLIB) > + find_package(ZLIB REQUIRED) > +ENDIF() push it in separate commit, please. > > option(USE_VALGRIND "Build to run safely under valgrind (often slower)." ON) > if(USE_VALGRIND) > Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp