- developers - lists.mariadb.org

Re: [Maria-developers] 2d73406dad8: MDEV-16481: set global system_versioning_asof=sf() crashes in specific case
by Sergei Golubchik 19 Oct '20

19 Oct '20

Hi, Nikita! ok to push (better late than never :) On Oct 19, Nikita Malyavin wrote: > revision-id: 2d73406dad8 (mariadb-10.3.16-39-g2d73406dad8) > parent(s): 609f358a9bd > author: Nikita Malyavin <nikitamalyavin(a)gmail.com> > committer: Sergei Golubchik <serg(a)mariadb.com> > timestamp: 2020-10-19 17:37:04 +0200 > message: > > MDEV-16481: set global system_versioning_asof=sf() crashes in specific case > > * sys_vars.h: add `MYSQL_TIME` field to `set_var::save_result` > * sys_vars.ic: get rid of calling `var->value->get_date()` from `Sys_var_vers_asof::update()` > * versioning.sysvars: add test; remove double warning > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] e3f45b2f9ea: MDEV-10267 Add ngram fulltext parser plugin
by Sergei Golubchik 19 Oct '20

19 Oct '20

Hi, Rinat! On Oct 19, Rinat Ibragimov wrote: > revision-id: e3f45b2f9ea (mariadb-10.2.31-368-ge3f45b2f9ea) > parent(s): 44c48cefdbe > author: Rinat Ibragimov <ri(a)tempesta-tech.com> > committer: Rinat Ibragimov <ri(a)tempesta-tech.com> > timestamp: 2020-08-25 01:59:06 +0300 > message: > > MDEV-10267 Add ngram fulltext parser plugin > > Ngram is a fulltext parser plugin that splits words into overlapping > segments of fixed lengths. For example, if 3-grams are in use, string > "abcdef" is split into "abc", "bcd", "cde", "def", thus allowing an > efficient substring search for CJK texts where splitting by words in not > feasible. Only word characters are going into n-grams. Spaces and > punctuation characters are treated as separators. > > diff --git a/plugin/fulltext/CMakeLists.txt b/plugin/fulltext/CMakeLists.txt > index 3c37666cb3e..9704733a9aa 100644 > --- a/plugin/fulltext/CMakeLists.txt > +++ b/plugin/fulltext/CMakeLists.txt > @@ -18,3 +18,7 @@ set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} -DNDEBUG") > > MYSQL_ADD_PLUGIN(ftexample plugin_example.c > MODULE_ONLY MODULE_OUTPUT_NAME "mypluglib" COMPONENT Test) > + > +# n-gram parser > +MYSQL_ADD_PLUGIN(ftngram ngram_parser/plugin_ngram.cc > + MODULE_ONLY MODULE_OUTPUT_NAME "fulltext_ngram") why to put the plugin under plugin/fulltext/ngram_parser/ ? it's only one file, just put it in plugin/fulltext/ > diff --git a/plugin/fulltext/mysql-test/ngram/disabled.def b/plugin/fulltext/mysql-test/ngram/disabled.def > new file mode 100644 > index 00000000000..0ca9dd33ef7 > --- /dev/null > +++ b/plugin/fulltext/mysql-test/ngram/disabled.def > @@ -0,0 +1 @@ > +ngram_gb18030 : GB18030 character set is not supported yet. I don't think there's any need to add tests for unsupported charsets. The plugin is trivial, no need to test it for every possible charset that we have and don't have. > diff --git a/plugin/fulltext/mysql-test/ngram/ngram_basic.inc b/plugin/fulltext/mysql-test/ngram/ngram_basic.inc > --- /dev/null > +++ b/plugin/fulltext/mysql-test/ngram/ngram_basic.inc > @@ -0,0 +1,25 @@ > +--echo > +--eval CREATE TABLE t (lang TEXT, str TEXT, FULLTEXT(str) WITH PARSER ngram) ENGINE=$engine CHARSET $charset > + > +# Some character are not possible to represent in some encodings. Ignore them. > +--disable_warnings > +--disable_query_log > +# http://www.gerolf.org/doc/polyglot/polyglot.htm. > +INSERT IGNORE INTO t VALUES > + ('Chinese', '他们为什么不说中文？'), > + ('English', 'Why can''t they just speak English?'), > + ('Japanese', 'なぜ、みんな日本語を話してくれないのか？'), > + ('Korean', '세계의 모든 사람들이 한국어를 이해한다면 얼마나 좋을까?'); > +--enable_query_log > +--enable_warnings > + > +SELECT * FROM t ORDER BY lang; > + > +SELECT * FROM t WHERE MATCH(str) AGAINST('之一' IN BOOLEAN MODE) ORDER BY lang; > +SELECT * FROM t WHERE MATCH(str) AGAINST('数据' IN BOOLEAN MODE) ORDER BY lang; > +SELECT * FROM t WHERE MATCH(str) AGAINST('zzz' IN BOOLEAN MODE) ORDER BY lang; > +SELECT * FROM t WHERE MATCH(str) AGAINST('gli' IN BOOLEAN MODE) ORDER BY lang; > +SELECT * FROM t WHERE MATCH(str) AGAINST('ない' IN BOOLEAN MODE) ORDER BY lang; > +SELECT * FROM t WHERE MATCH(str) AGAINST('람들' IN BOOLEAN MODE) ORDER BY lang; > + > +DROP TABLE t; > diff --git a/plugin/fulltext/mysql-test/ngram/ngram_basic.test b/plugin/fulltext/mysql-test/ngram/ngram_basic.test > new file mode 100644 > index 00000000000..eaa8829c3ee > --- /dev/null > +++ b/plugin/fulltext/mysql-test/ngram/ngram_basic.test > @@ -0,0 +1,25 @@ > +# Ngram fulltext parser basic functionality. > + > +SET NAMES utf8mb4; > +SELECT @@ngram_token_size; > + > +let $engine_index= 3; > +while ($engine_index) { > + if ($engine_index == 3) { let $engine= InnoDB; } > + if ($engine_index == 2) { let $engine= MyISAM; } > + if ($engine_index == 1) { let $engine= Aria; } > + > + let $charset_index= 6; > + while ($charset_index) { > + if ($charset_index == 6) { let $charset= utf8mb4; } > + if ($charset_index == 5) { let $charset= big5; } > + if ($charset_index == 4) { let $charset= gbk; } > + if ($charset_index == 3) { let $charset= gb2312; } > + if ($charset_index == 2) { let $charset= sjis; } > + if ($charset_index == 1) { let $charset= ujis; } > + > + source ngram_basic.inc; > + dec $charset_index; > + } eh. unroll the loop please, write let $engine= InnoDB; let $charset= utf8mb4; source ngram_basic.inc; let $charset= big5; source ngram_basic.inc; ... Or, better: CREATE TABLE t (lang TEXT, str TEXT, FULLTEXT(str) WITH PARSER ngram) ENGINE=InnoDB CHARSET utf8mb4; source ngram_basic.inc; CREATE TABLE t (lang TEXT, str TEXT, FULLTEXT(str) WITH PARSER ngram) ENGINE=InnoDB CHARSET big5; source ngram_basic.inc; ... > + dec $engine_index; > +} > diff --git a/plugin/fulltext/mysql-test/ngram/ngram_missing_plugin.result b/plugin/fulltext/mysql-test/ngram/ngram_missing_plugin.result > new file mode 100644 > index 00000000000..aa17b1d6053 > --- /dev/null > +++ b/plugin/fulltext/mysql-test/ngram/ngram_missing_plugin.result > @@ -0,0 +1,23 @@ there are tests for the missing fulltest parser plugin. they don't depend on the individial plugin (that is, btw, *not installed*). No need to repeat them for every plugin, remove this test. > +SET NAMES utf8mb4; > +CREATE TABLE t (str TEXT, FULLTEXT(str) WITH PARSER ngram) > +ENGINE=InnoDB; > +INSERT INTO t VALUES ('Hello World'), ('abcdef ghijklm'); > +# Restart with ngram plugin disabled. > +SET NAMES utf8mb4; > +SELECT * FROM t WHERE MATCH(str) AGAINST('bcd') ORDER BY str; > +ERROR HY000: Plugin 'ngram' is not loaded > +SELECT * FROM t ORDER BY str; > +ERROR HY000: Plugin 'ngram' is not loaded > +INSERT INTO t VALUES ('something'); > +ERROR HY000: Plugin 'ngram' is not loaded > +# Restart with ngram plugin enabled again. > +SET NAMES utf8mb4; > +SELECT * FROM t WHERE MATCH(str) AGAINST('bcd') ORDER BY str; > +str > +abcdef ghijklm > +SELECT * FROM t ORDER BY str; > +str > +abcdef ghijklm > +Hello World > +INSERT INTO t VALUES ('something'); > +DROP TABLE t; > diff --git a/plugin/fulltext/mysql-test/ngram/ngram_token_size.result b/plugin/fulltext/mysql-test/ngram/ngram_token_size.result > new file mode 100644 > index 00000000000..7043e147d5b > --- /dev/null > +++ b/plugin/fulltext/mysql-test/ngram/ngram_token_size.result > @@ -0,0 +1,109 @@ > +# Check 'ngram_token_size' variable. > +# > +# It's a global variable. > +SELECT @@global.ngram_token_size; > +@@global.ngram_token_size > +2 > +SHOW GLOBAL VARIABLES LIKE 'ngram_token_size'; > +Variable_name Value > +ngram_token_size 2 > +SELECT * FROM information_schema.global_variables WHERE variable_name='ngram_token_size'; > +VARIABLE_NAME VARIABLE_VALUE > +NGRAM_TOKEN_SIZE 2 > +# > +# Not a session variable. > +SELECT @@session.ngram_token_size; > +ERROR HY000: Variable 'ngram_token_size' is a GLOBAL variable > +SHOW SESSION VARIABLES LIKE 'ngram_token_size'; > +Variable_name Value > +ngram_token_size 2 > +SELECT * FROM information_schema.session_variables WHERE variable_name='ngram_token_size'; > +VARIABLE_NAME VARIABLE_VALUE > +NGRAM_TOKEN_SIZE 2 > +# > +# 0 is outside allowed range [1, 10], so it's changed to 1. > +SET GLOBAL ngram_token_size= 0; > +Warnings: > +Warning 1292 Truncated incorrect ngram_token_size value: '0' > +SELECT * FROM information_schema.global_variables WHERE variable_name='ngram_token_size'; > +VARIABLE_NAME VARIABLE_VALUE > +NGRAM_TOKEN_SIZE 1 > +# > +# 10 is an acceptable value. > +SET GLOBAL ngram_token_size= 10; > +SELECT * FROM information_schema.global_variables WHERE variable_name='ngram_token_size'; > +VARIABLE_NAME VARIABLE_VALUE > +NGRAM_TOKEN_SIZE 10 > +# > +# 1 is an acceptable value. > +SET GLOBAL ngram_token_size= 1; > +SELECT * FROM information_schema.global_variables WHERE variable_name='ngram_token_size'; > +VARIABLE_NAME VARIABLE_VALUE > +NGRAM_TOKEN_SIZE 1 > +# > +# 11 is outside allowed range [1, 10], so it's changed to 10. > +SET GLOBAL ngram_token_size= 11; > +Warnings: > +Warning 1292 Truncated incorrect ngram_token_size value: '11' > +SELECT * FROM information_schema.global_variables WHERE variable_name='ngram_token_size'; > +VARIABLE_NAME VARIABLE_VALUE > +NGRAM_TOKEN_SIZE 10 ^^^^ all tests above aren't needed, it's enough to do query_vertical select * from information_schema.system_variables where variable_name='ngram_token_size'; all the behavior above is implemented outside of the plugin, no need to test it. > +# > +# Try 3-grams. > +SET GLOBAL ngram_token_size= 3; > +CREATE TABLE t (str TEXT, FULLTEXT INDEX (str) WITH PARSER ngram) Engine=InnoDB; > +INSERT INTO t VALUES > +('Lorem ipsum dolor sit amet, consectetur adipiscing elit.'), > +('Maecenas non pellentesque odio.'), > +('Sed fermentum egestas sapien vel consequat.'); > +# Only one row contains substring 'api'. > +SELECT str FROM t WHERE MATCH(str) AGAINST('api'); > +str > +Sed fermentum egestas sapien vel consequat. > +# 'lent' is parsed into ('len', 'ent') list. Two rows contain at least one of the tokens. > +SELECT str FROM t WHERE MATCH(str) AGAINST('lent*'); > +str > +Maecenas non pellentesque odio. > +Sed fermentum egestas sapien vel consequat. > +# 'zzz' is not present anywhere, so result is empty. > +SELECT str FROM t WHERE MATCH(str) AGAINST('zzzz'); > +str > +# 'piz' cannot be found anywhere, but 'api' is in one of the rows. > +SELECT str FROM t WHERE MATCH(str) AGAINST('apiz'); > +str > +Sed fermentum egestas sapien vel consequat. > +# > +# Try switching to 2-grams while keeping the index. > +SET GLOBAL ngram_token_size= 2; > +# 'api' is now parsed into ('ap', 'pi'). Since index only contains 3-grams, nothing is found. > +SELECT str FROM t WHERE MATCH(str) AGAINST('api'); > +str > +# > +# Add some data while ngram size is set to 2. > +INSERT INTO t VALUES('1234 56789'); > +# > +# Now index contains 3-grams from the rows added before and a number of > +# 2-grams from the just added row. > +# Querying for 'api' still returns nothing, but line with '678' is found. > +SELECT str from t WHERE MATCH(str) AGAINST('api'); > +str > +SELECT str FROM t WHERE MATCH(str) AGAINST('678'); > +str > +1234 56789 > +# > +# Switching back to 3-grams make intitially added rows searchable. > +SET GLOBAL ngram_token_size= 3; > +SELECT str FROM t WHERE MATCH(str) AGAINST('api'); > +str > +Sed fermentum egestas sapien vel consequat. > +# > +# But row with '678' in it cannot be found, as it was added with 2-grams. > +SELECT str FROM t WHERE MATCH(str) AGAINST('678'); > +str > +# > +# Try matching in boolean mode. > +SELECT str FROM t WHERE MATCH(str) AGAINST('+ent -api' IN BOOLEAN MODE); > +str > +Maecenas non pellentesque odio. > +# > +DROP TABLE t; > diff --git a/plugin/fulltext/mysql-test/ngram/ngram_ucs2.result b/plugin/fulltext/mysql-test/ngram/ngram_ucs2.result > new file mode 100644 > index 00000000000..9d82743bd24 > --- /dev/null > +++ b/plugin/fulltext/mysql-test/ngram/ngram_ucs2.result > @@ -0,0 +1,8 @@ > +CREATE TABLE t (str TEXT, FULLTEXT INDEX (str) WITH PARSER ngram) Engine=InnoDB CHARSET=UCS2; > +ERROR HY000: Column 'str' cannot be part of FULLTEXT index > +CREATE TABLE t (str TEXT, FULLTEXT INDEX (str) WITH PARSER ngram) Engine=InnoDB CHARSET=UTF16; > +ERROR HY000: Column 'str' cannot be part of FULLTEXT index > +CREATE TABLE t (str TEXT, FULLTEXT INDEX (str) WITH PARSER ngram) Engine=InnoDB CHARSET=UTF16LE; > +ERROR HY000: Column 'str' cannot be part of FULLTEXT index > +CREATE TABLE t (str TEXT, FULLTEXT INDEX (str) WITH PARSER ngram) Engine=InnoDB CHARSET=UTF32; > +ERROR HY000: Column 'str' cannot be part of FULLTEXT index same as above. This is fulltext index limitation, it does not depend on the parser plugin. The plugin doesn't need to test for it. > diff --git a/plugin/fulltext/mysql-test/ngram/suite.opt b/plugin/fulltext/mysql-test/ngram/suite.opt > new file mode 100644 > index 00000000000..ab37cea996e > --- /dev/null > +++ b/plugin/fulltext/mysql-test/ngram/suite.opt > @@ -0,0 +1,4 @@ > +--plugin-load-add=$FULLTEXT_NGRAM_SO > +--loose-innodb no need to enable innodb explicitly, `source have_innodb.inc` does that. > +--loose-enable-innodb_ft_index_table > +--loose-innodb_ft_index_cache why did you enable these two plugins if you don't use them in your tests? > diff --git a/plugin/fulltext/ngram_parser/plugin_ngram.cc b/plugin/fulltext/ngram_parser/plugin_ngram.cc > new file mode 100644 > index 00000000000..80498c3299b > --- /dev/null > +++ b/plugin/fulltext/ngram_parser/plugin_ngram.cc > @@ -0,0 +1,256 @@ > +/* > + Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved. > + Copyright (c) 2020, MariaDB. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License, version 2.0, > + as published by the Free Software Foundation. > + > + This program is also distributed with certain software (including > + but not limited to OpenSSL) that is licensed under separate terms, > + as designated in a particular file or component or in included license > + documentation. The authors of MySQL hereby grant you an additional > + permission to link the program and your derivative works with the > + separately licensed software that they have included with MySQL. > + > + This program 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, version 2.0, 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 St, Fifth Floor, Boston, MA 02110-1301 USA */ > + > +#include <stddef.h> > +#include <limits.h> > +#include <my_attribute.h> > +#include <my_global.h> > +#include "../storage/innobase/include/fts0tokenize.h" No, the plugin should be self-contained and not include anything from other plugins. At least if a plugin is as trivial as that one. > + > +#define U_LOW_LINE 0x5f // Code point for underscore '_'. > +#define is_alnum(ct) ((ct) & (_MY_U | _MY_L | _MY_NMR)) > + > +static int ngram_token_size; > + > +static MYSQL_SYSVAR_INT(token_size, ngram_token_size, > + 0, > + "Ngram full text plugin parser token size in characters", > + NULL, NULL, 2, 1, 10, 0); > + > +static int my_ci_charlen(CHARSET_INFO *cs, const uchar *str, const uchar *end) > +{ > + return my_charlen(cs, reinterpret_cast<const char *>(str), > + reinterpret_cast<const char *>(end)); > +} > + > +static int my_ci_ctype(CHARSET_INFO *cs, int *char_type, const uchar *str, > + const uchar *end) > +{ > + return cs->cset->ctype(cs, char_type, str, end); > +} > + > +static bool is_underscore(CHARSET_INFO *cs, const uchar *str, const uchar *end) > +{ > + my_wc_t wc; > + cs->cset->mb_wc(cs, &wc, str, end); > + return wc == U_LOW_LINE; > +} > + > +// Splits a string into ngrams and emits them. > +static int split_into_ngrams(MYSQL_FTPARSER_PARAM *param, const uchar *doc, > + int len, MYSQL_FTPARSER_BOOLEAN_INFO *bool_info) > +{ > + const CHARSET_INFO *cs= param->cs; > + const uchar *start= doc; > + const uchar *end= doc + len; > + const uchar *next= start; > + int n_chars= 0; > + int ret= 0; > + bool is_first= true; > + > + while (next < end) { > + int char_type; > + int char_len= my_ci_ctype(cs, &char_type, next, end); > + > + // Broken data? > + if (next + char_len > end || char_len == 0) > + break; > + > + // Avoid creating partial n-grams by stopping at word boundaries. > + // Underscore is treated as a word character too, since identifiers in > + // programming languages often contain them. > + if (!is_alnum(char_type) && !is_underscore(cs, next, end)) { > + start= next + char_len; > + next= start; > + n_chars= 0; > + continue; I don't think it's a good idea. On the opposite, I would rather create ngrams over word boundaries, so that, say, I'd split "n-gram plugin" to "n-g", "-gr", "gra", "ram", "am ", "m p", " pl", "plu", "lug", "ugi", "gin". May be better would be to split it into "n-gr", "gra", "ram", "am p", "m pl", "plu", "lug", "ugi", "gin". (that is, always three letters in an n-gram, not three characters). This needs testing to find out what approach works better. > + } > + > + next += char_len; > + n_chars++; > + > + if (n_chars == ngram_token_size) { > + bool_info->position= static_cast<uint>(start - doc); nope, we don't have MYSQL_FTPARSER_FULL_BOOLEAN_INFO::position and this plugin is definitely not the reason to break the API and add it. > + ret= param->mysql_add_word(param, reinterpret_cast<const char *>(start), > + static_cast<int>(next - start), bool_info); > + if (ret != 0) > + return ret; > + > + start += my_ci_charlen(cs, start, end); why do you scan the string again here, you've already counted character by character above? > + n_chars= ngram_token_size - 1; > + is_first= false; > + } > + } > + > + // If nothing was emitted yet, emit the remainder. what reminder, what is it for? > + switch (param->mode) { > + case MYSQL_FTPARSER_FULL_BOOLEAN_INFO: > + case MYSQL_FTPARSER_WITH_STOPWORDS: > + if (n_chars > 0 && is_first) { > + assert(next > start); > + assert(n_chars < ngram_token_size); > + > + bool_info->position= static_cast<uint>(start - doc); > + ret= param->mysql_add_word(param, reinterpret_cast<const char *>(start), > + static_cast<int>(next - start), bool_info); > + } > + break; > + > + default: > + break; > + } > + > + return ret; > +} > + > +static int number_of_chars(const CHARSET_INFO *cs, const uchar *buf, int len) > +{ > + const uchar *ptr= buf; > + const uchar *end= buf + len; > + int size= 0; > + > + while (ptr < end) { > + ptr += my_ci_charlen(cs, ptr, end); > + size += 1; > + } > + > + return size; > +} > + > +// Convert term into phrase and handle wildcard. > +static int ngram_term_convert(MYSQL_FTPARSER_PARAM *param, const uchar *token, > + int len, MYSQL_FTPARSER_BOOLEAN_INFO *bool_info) > +{ > + MYSQL_FTPARSER_BOOLEAN_INFO token_info= > + { FT_TOKEN_WORD, 0, 0, 0, 0, 0, ' ', 0 }; > + const CHARSET_INFO *cs= param->cs; > + int ret= 0; > + > + assert(bool_info->type == FT_TOKEN_WORD); > + assert(bool_info->quot == NULL); > + > + /* Convert rules: > + * 1. if term with wildcard and term length is less than ngram_token_size, > + * we keep it as normal term search. > + * 2. otherwise, term is converted to phrase and wildcard is ignored. > + * e.g. 'abc' and 'abc*' are both equivalent to '"ab bc"'. > + */ > + > + if (bool_info->trunc && number_of_chars(cs, token, len) < ngram_token_size) { > + ret= param->mysql_add_word(param, reinterpret_cast<const char *>(token), > + len, bool_info); > + } else { > + bool_info->type= FT_TOKEN_LEFT_PAREN; > + bool_info->quot= reinterpret_cast<char*>(1); > + > + ret= param->mysql_add_word(param, NULL, 0, bool_info); > + if (ret != 0) > + return ret; > + > + ret= split_into_ngrams(param, token, len, &token_info); > + if (ret != 0) > + return ret; > + > + bool_info->type= FT_TOKEN_RIGHT_PAREN; > + ret= param->mysql_add_word(param, NULL, 0, bool_info); > + > + assert(bool_info->quot == NULL); > + bool_info->type= FT_TOKEN_WORD; > + } > + > + return ret; > +} > + > +static int ngram_parser_parse(MYSQL_FTPARSER_PARAM *param) > +{ > + MYSQL_FTPARSER_BOOLEAN_INFO bool_info= > + { FT_TOKEN_WORD, 0, 0, 0, 0, 0, ' ', 0 }; > + const CHARSET_INFO *cs= param->cs; > + uchar *start= reinterpret_cast<uchar *>(const_cast<char *>(param->doc)); > + uchar *end= start + param->length; > + FT_WORD word= {NULL, 0, 0}; > + int ret= 0; > + > + switch (param->mode) { > + case MYSQL_FTPARSER_SIMPLE_MODE: > + case MYSQL_FTPARSER_WITH_STOPWORDS: > + ret= split_into_ngrams(param, start, static_cast<int>(end - start), > + &bool_info); > + break; > + > + case MYSQL_FTPARSER_FULL_BOOLEAN_INFO: > + // Reusing InnoDB's boolean mode parser to handle all special characters. > + while (fts_get_word(cs, &start, end, &word, &bool_info)) { nope. InnoDB might not even be compiled it. Use param->mysql_parse() > + if (bool_info.type == FT_TOKEN_WORD) { > + if (bool_info.quot != NULL) { > + ret= split_into_ngrams(param, word.pos, word.len, &bool_info); > + } else { > + ret= ngram_term_convert(param, word.pos, word.len, &bool_info); > + } > + } else { > + ret= param->mysql_add_word(param, reinterpret_cast<char*>(word.pos), > + word.len, &bool_info); > + } > + > + if (ret != 0) > + return ret; > + } > + > + break; > + } > + > + return ret; > +} > + > +static struct st_mysql_ftparser ngram_parser_descriptor= > +{ > + MYSQL_FTPARSER_INTERFACE_VERSION, > + ngram_parser_parse, > + NULL, > + NULL, > +}; > + > +static struct st_mysql_sys_var *ngram_system_variables[]= > +{ > + MYSQL_SYSVAR(token_size), > + NULL > +}; > + > +maria_declare_plugin(ngram_parser) > +{ > + MYSQL_FTPARSER_PLUGIN, // Type. > + &ngram_parser_descriptor, // Descriptor. > + "ngram", // Name. > + "Oracle Corp", // Author. > + "Ngram Full-Text Parser", // Description. > + PLUGIN_LICENSE_GPL, // License. > + NULL, // Initialization function. > + NULL, // Deinitialization function. > + 0x0100, // Numeric version. > + NULL, // Status variables. > + ngram_system_variables, // System variables. > + "1.0", // String version representation. > + MariaDB_PLUGIN_MATURITY_EXPERIMENTAL, // Maturity. > +} > +maria_declare_plugin_end; > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 12d5c4f2813: Fixed problems with S3 after "DROP TABLE FORCE" changes
by Sergei Golubchik 19 Oct '20

19 Oct '20

Hi, Monty! On Oct 16, Michael Widenius wrote: > revision-id: 12d5c4f2813 (mariadb-10.5.4-190-g12d5c4f2813) > parent(s): afa5f735f30 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2020-09-13 17:48:15 +0300 > message: > > Fixed problems with S3 after "DROP TABLE FORCE" changes > > MDEV-23691 S3 storage engine: delayed slave can drop the table please put the MDEV on the first line of the commit comment > > This commit also fixes all failing replication S3 tests. > > A slave is delayed if it is trying to execute replicated queries on a > table that is already converted to S3 by the master. What do you mean, a slave is delayed? > > Fixes for replication events on S3 tables for delayed slaves: > - INSERT and INSERT ... SELECT and CREATE TABLE are ignored but written > to the binary log. UPDATE & DELETE will be fixed in a future commit. > > Other things: > - On slaves with --s3-slave-ignore-updates set, allow S3 tables to be > opened in read-write mode. This was done to be able to > ignore-but-replicate queries like insert. Without this change any > open of an S3 table failed with 'Table is read only' which is too > early to be able to replicate the original query. > - Errors are now printed if handler::extra() call fails in > wait_while_tables_are_used() > - Error message for row changes are changed from HA_ERR_WRONG_COMMAND > to HA_ERR_TABLE_READONLY > - Disable some maria_extra() calls for S3 tables. This could cause > S3 tables to fail in some cases. > - Added missing thr_lock_delete() to ma_open() in case of failure > > diff --git a/sql/log.cc b/sql/log.cc > index 2a887a68606..9c656387e90 100644 > --- a/sql/log.cc > +++ b/sql/log.cc > @@ -2151,6 +2151,12 @@ static int binlog_commit(handlerton *hton, THD *thd, bool all) > > DBUG_RETURN(0); > } > + /* > + This is true if we are doing an alter table that is replicated as > + CREATE TABLE ... SELECT > + */ > + if (thd->variables.option_bits & OPTION_BIN_COMMIT_OFF) > + DBUG_RETURN(0); why binlog_commit is even called here? > > DBUG_PRINT("debug", > ("all: %d, in_transaction: %s, all.modified_non_trans_table: %s, stmt.modified_non_trans_table: %s", > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index f4cbda5490c..7c103756566 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -7476,6 +7476,34 @@ int THD::binlog_query(THD::enum_binlog_query_type qtype, char const *query_arg, > DBUG_RETURN(0); > } > > + > +/** > + Binlog current query as a statement if binary log is open > + > + @retval false ok > + @retval true error > +*/ > + > + > +bool THD::binlog_current_query() > +{ > + if (!mysql_bin_log.is_open()) > + return 0; > + > + /* > + The following is needed if the query was "ignored" by a shared distributed > + engine, in which case decide_logging_format() will set the binlog filter > + */ > + reset_binlog_local_stmt_filter(); > + clear_binlog_local_stmt_filter(); > + return binlog_query(THD::STMT_QUERY_TYPE, query(), query_length(), > + /* is_trans */ FALSE, > + /* direct */ FALSE, > + /* suppress_use */ FALSE, > + /* Error */ 0) > 0; > +} you created a generally looking function with a rather special behavior. Please rename it to make sure it's not a general purpose helper. For example, call it binlog_current_query_unfiltered() or force_binlog_current_query(). And the function comment should say Force-binlog current query as a statement ignoring the binlog filter setting > + > + > void > THD::wait_for_wakeup_ready() > { > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc > index 4ad4c478937..5f59e064a35 100644 > --- a/sql/sql_insert.cc > +++ b/sql/sql_insert.cc > @@ -724,6 +724,7 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, > Item *unused_conds= 0; > DBUG_ENTER("mysql_insert"); > > + bzero((char*) &info,sizeof(info)); why did you move it up? > create_explain_query(thd->lex, thd->mem_root); > /* > Upgrade lock type if the requested lock is incompatible with > @@ -764,16 +765,27 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, > DBUG_RETURN(TRUE); > value_count= values->elements; > > - if (mysql_prepare_insert(thd, table_list, table, fields, values, > - update_fields, update_values, duplic, > - &unused_conds, FALSE)) > + if ((res= mysql_prepare_insert(thd, table_list, table, fields, values, > + update_fields, update_values, duplic, > + &unused_conds, FALSE))) > + { > + retval= thd->is_error(); > + if (res < 0) > + { > + /* > + Insert should be ignored but we have to log the query in statement > + format in the binary log > + */ > + res= thd->binlog_current_query(); abort label doesn't look at `res` it returns `retval`. So I don't think you need `retval= thd->is_error()` at all, and instead you need if (res < 0) retval= thd->binlog_current_query(); > + } > goto abort; > + } > + /* mysql_prepare_insert sets table_list->table if it was not set */ > + table= table_list->table; > > /* Prepares LEX::returing_list if it is not empty */ > if (returning) > result->prepare(returning->item_list, NULL); > - /* mysql_prepare_insert sets table_list->table if it was not set */ > - table= table_list->table; why did you move this up? > > context= &thd->lex->first_select_lex()->context; > /* > @@ -1575,21 +1587,29 @@ bool mysql_prepare_insert(THD *thd, TABLE_LIST *table_list, > DBUG_ASSERT (!select_insert || !values); > > if (mysql_handle_derived(thd->lex, DT_INIT)) > DBUG_RETURN(1); > if (table_list->handle_derived(thd->lex, DT_MERGE_FOR_INSERT)) > DBUG_RETURN(1); > if (thd->lex->handle_list_of_derived(table_list, DT_PREPARE)) > DBUG_RETURN(1); > > if (duplic == DUP_UPDATE) > { > /* it should be allocated before Item::fix_fields() */ > if (table_list->set_insert_values(thd->mem_root)) > DBUG_RETURN(1); > + } > + > + if (!table) > + table= table_list->table; As far as I can see, table in mysql_prepare_insert is *always* either table_list->table or NULL. At least in 10.5. So I'd rather remove the table argument completely and let mysql_prepare_insert always use table_list->table. > + > + if (table->file->check_if_updates_are_ignored("INSERT")) > + { > + DBUG_RETURN(-1); > } > > if (mysql_prepare_insert_check_table(thd, table_list, fields, select_insert)) > DBUG_RETURN(1); > > /* Prepare the fields in the statement. */ > if (values) > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index 15d190c3139..2843496f783 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -5206,7 +5215,19 @@ int create_table_impl(THD *thd, const LEX_CSTRING &orig_db, > goto err; > } > else if (options.if_not_exists()) > + { > + /* > + We never come here as part of normal create table as table existance > + is checked in open_and_lock_tables(). We may come here as part of > + ALTER TABLE > + */ How so? There is no ALTER IF NOT EXISTS. And this is the old code, so it seems to be for CREATE TABLE IF NOT EXISTS. > + > + /* Log CREATE IF NOT EXISTS on slave for distributed engines */ > + if (thd->slave_thread && (exists_hton && exists_hton->flags & > + HTON_IGNORE_UPDATES)) > + thd->log_current_statement= 1; > goto warn; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] MDEV-21265: IN predicate conversion to IN subquery should be allowed for a broader set of datatype comparison
by Sergey Petrunia 16 Oct '20

16 Oct '20

Hi Varun, > Commit 3e29c5a1620deef2fa9b25a0d42b4c07fe59e96f > > MDEV-21265: IN predicate conversion to IN subquery should be allowed for a broader set of datatype comparison > > Allow materialization strategy when the collation of the columns of the > left side of the IN subquery are not same as the one on the right side > of the IN subquery but are compatible with each other. > This allows conversion from utf8mb3 to utf8mb4 as the former is a subset of the later. Please fix the comment to say what the patch does: When collations on the inner and outer sides are the same, and the character set of the inner side is a proper subset of the charset on the outer. > > diff --git a/mysql-test/main/subselect4.result b/mysql-test/main/subselect4.result > index 020a70642913..f3ae2398d45e 100644 > --- a/mysql-test/main/subselect4.result > +++ b/mysql-test/main/subselect4.result > @@ -2718,3 +2718,172 @@ Warning 1931 Query execution was interrupted. The query examined at least 3020 r > SET join_cache_level= @save_join_cache_level; > DROP TABLE t1,t2,t3,t4; > # End of 10.2 tests > +# > +# MDEV-21265: IN predicate conversion to IN subquery should be allowed for a broader set of datatype comparison > +# > +CREATE TABLE t1 ( col1 varchar (32) collate utf8_general_ci); > +CREATE TABLE t2 ( col1 varchar (32) collate utf8mb4_general_ci, key(col1)); > +INSERT INTO t1 select seq from seq_1_to_100; > +INSERT INTO t2 values (_utf8mb4 X'F09F9883'), (1),('?'); ... > EXPLAIN FORMAT=JSON SELECT HEX(col1), col1 IN (SELECT col1 FROM t1) FROM t2; Please insert the '?' into the table t1, not t2. The idea of the check is to check for the following: - We get smiley symbol from table t2. - We make the lookup key from it. This results in an error and the '?' as output. - Then, a lookup is made in t1. And we are trying to provoke a wrong result by having the '?' in t1. OK to push into the custom-build tree when the above is addressed. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] 0d05c9f8699: Add mytop to Client component for rpm package
by Sergei Golubchik 13 Oct '20

13 Oct '20

Hi, Anel! About the diff to 1.9, I tend to agree that our mytop is a superset. I did have some comments when looking at the diff, couldn't help it :) Here they are: > diff --git a/scripts/mytop.sh b/../mytop-1.9.1-4 > index 321b79537fc..056b7a9a806 100644 > --- b/../mytop-1.9.1-4 > +++ a/scripts/mytop.sh > @@ -1,13 +1,13 @@ > -#!/usr/bin/perl -w > +#!/usr/bin/env perl I thought we're getting rid of "env perl" ? > # > -# $Id: mytop,v 1.91 2012/01/18 16:49:12 mgrennan Exp $ > +# $Id: mytop,v 1.99-maria6 2019/10/22 14:53:51 jweisbuch Exp $ > > =pod > > =head1 NAME > > -mytop - display MySQL server performance info like `top' > +mytop - display MariaDB server performance info like `top' may be "MariaDB/MySQL" here and below? > > =cut > > @@ -242,11 +253,15 @@ my $dsn; > > ## Socket takes precedence. > > -$dsn ="DBI:mysql:database=$config{db};mysql_read_default_group=mytop;"; > +if (eval {DBI->install_driver("MariaDB")}) { > + $dsn = "DBI:MariaDB:database=$config{db};mariadb_read_default_group=mytop;"; > +} else { > + $dsn = "DBI:mysql:database=$config{db};mysql_read_default_group=mytop;"; > +} > > if ($config{socket} and -S $config{socket}) > { > - $dsn .= "mysql_socket=$config{socket}"; > + $dsn .= "mariadb_socket=$config{socket}"; if you support both drivers, you need to use the correct option here > } > else > { > @@ -1027,10 +918,15 @@ sub GetData() > } > } > > - open L, "</proc/loadavg"; > - my $l = <L>; > - close L; > - chomp $l; > + my $l; > + if (-e "/proc/loadavg") > + { > + ## To avoid warnings if the OS is not Linux > + open (my $fh, "<", "/proc/loadavg"); > + ## Only the first 3 values are interresting rr? > + $l = join(" ", (split /\s+/, <$fh>)[0..2]); > + close $fh; > + } > > $last_time = $now_time; > > @@ -1960,11 +1694,15 @@ sub PrintHelp() > S - change slow query hightlighting > t - switch to thread view (default) > u - show only a specific user > + V - show variables > : - enter a command (not yet implemented) > ! - Skip an error that has stopped replications (at your own risk) > + L - show full queries (do not strip to terminal width) > + w - adjust the User and DB columns width > + a - toggle the progress column > > -${GREEN}http://jeremy.zawodny.com/mysql/mytop/${RESET} > -${GREEN}http://www.mysqlfanboy.com/mytop-3/${RESET} > +Base version from ${GREEN}http://www.mysqlfanboy.com/mytop-3${RESET} if it's mytop-3, perhaps it should have the version 3.x and not 1.99-maria6 ? > +This version comes as part of the ${GREEN}MariaDB${RESET} distribution. > ]; > > print $help; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

3 2

Re: [Maria-developers] d19f380d62c: MDEV-21445 Strange/inconsistent behavior of IN condition when mixing numbers and strings
by Sergei Golubchik 12 Oct '20

12 Oct '20

Hi, Alexander! On Oct 12, Alexander Barkov wrote: > revision-id: d19f380d62c (mariadb-10.3.21-213-gd19f380d62c) > parent(s): 90c8d773ed0 > author: Alexander Barkov <bar(a)mariadb.com> > committer: Alexander Barkov <bar(a)mariadb.com> > timestamp: 2020-08-18 13:19:21 +0400 > message: > > MDEV-21445 Strange/inconsistent behavior of IN condition when mixing numbers and strings > > Comparison is now performed as DECIMAL for pairs (int,string) and (string,int). Are there tests with strings like '1e100', outside of bigint range ? > Before the patch the comparison was performed in DOUBLE format, > so precision loss happened on huge values, e.g.: > > '97716021308405775' was converted to > 97716021308405770 (notice, the last digit was lost) > > diff --git a/sql/field.h b/sql/field.h > index be3a648617b..c9f5a34a3ae 100644 > --- a/sql/field.h > +++ b/sql/field.h > @@ -311,7 +311,7 @@ class Value_source > const char *end) > { > Converter_str2my_decimal_with_warn(NULL, Warn_filter_all(), > - E_DEC_FATAL_ERROR & ~E_DEC_BAD_NUM, > + 0, why did you remove warnings (here and elsewhere)? would be good to have it in the commit comment too, not just in email. > cs, cptr, end - cptr, decimal_value); > return decimal_value; > } > diff --git a/sql/sql_type.cc b/sql/sql_type.cc > index 9d3a47adfa5..7a847c90b8f 100644 > --- a/sql/sql_type.cc > +++ b/sql/sql_type.cc > @@ -764,6 +764,11 @@ Type_handler_hybrid_field_type::aggregate_for_comparison(const Type_handler *h) > m_type_handler= &type_handler_datetime; > } > } > + else if ((a == INT_RESULT && b == STRING_RESULT) || > + (b == INT_RESULT && a == INT_RESULT)) looks like a typo, the second line should be b == INT_RESULT && a == STRING_RESULT please add some tests to cover it. > + { > + m_type_handler= &type_handler_newdecimal; > + } > else if ((a == INT_RESULT || a == DECIMAL_RESULT) && > (b == INT_RESULT || b == DECIMAL_RESULT)) > { Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 2792940391a: MDEV-23327 Can't uninstall UDF if the implementation library file doesn't exist
by Sergei Golubchik 11 Oct '20

11 Oct '20

Hi, Oleksandr! Mostly ok, one problem below: On Oct 10, Oleksandr Byelkin wrote: > revision-id: 2792940391a (mariadb-10.2.31-343-g2792940391a) > parent(s): 2107e3bb9c0 > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2020-07-29 21:54:24 +0200 > message: > > MDEV-23327 Can't uninstall UDF if the implementation library file doesn't exist > > Made cleanup of DROP (udf) FUNCTION procedure and also check of mysql.func (not only loaded udf). > > @@ -606,43 +648,54 @@ int mysql_create_function(THD *thd,udf_func *udf) > } > > > -int mysql_drop_function(THD *thd,const LEX_STRING *udf_name) > +enum drop_udf_result mysql_drop_function(THD *thd, const LEX_STRING *udf_name) > { > TABLE *table; > - TABLE_LIST tables; > udf_func *udf; > DBUG_ENTER("mysql_drop_function"); > > + if (!(table= open_udf_func_table(thd))) > + DBUG_RETURN(UDF_DEL_RESULT_ERROR); > + > + // Pre-check without protection > + if (!find_udf_everywhere(thd, udf_name->str, udf_name->length, table)) you cannot check the udf_hash without a lock > + { > + close_mysql_tables(thd); Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

[Maria-developers] Unsubscribe
by TINKU BANSAL 11 Oct '20

11 Oct '20

Hi there, Please Unsubscribe me to this mailing list. Thanks

2 1

Re: [Maria-developers] 62c35fe93e1: MDEV-15703 Crash in EXECUTE IMMEDIATE 'CREATE OR REPLACE TABLE t1 (a INT DEFAULT ?)' USING DEFAULT
by Sergei Golubchik 09 Oct '20

09 Oct '20

Hi, Oleksandr! On Oct 09, Oleksandr Byelkin wrote: > revision-id: 62c35fe93e1 (mariadb-10.2.31-487-g62c35fe93e1) > parent(s): f584d567dbb > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2020-10-07 13:43:39 +0200 > message: > > MDEV-15703 Crash in EXECUTE IMMEDIATE 'CREATE OR REPLACE TABLE t1 (a INT DEFAULT ?)' USING DEFAULT > > part 2: > > - check that expressions are evaluable for > making empty row and assigning PS variable > > - correctly handling writing to a temporary tabe during multi-update > by setting associated field > > - Item::raise_error_not_evaluable() bakported from 10.4 and made vitrual > > - Item::is_evaluable_expression() bakported from 10.4 > > - Item::check_is_evaluable_expression_or_error() bakported from 10.4 I still don't quite understand all details of your patch. But here're more questions and comments below: > diff --git a/sql/field.cc b/sql/field.cc > index bdaaecc2026..8ce412a207d 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -11183,6 +11205,16 @@ bool Field::save_in_field_default_value(bool view_error_processing) > { > THD *thd= table->in_use; > > + if ( // table is not opened properly > + table == NULL || table->pos_in_table_list == NULL || > + table->pos_in_table_list->select_lex == NULL || > + // we are in subquery/derived/CTE > + !table->pos_in_table_list->top_table()->select_lex->is_top_level_select()) > + { > + DBUG_ASSERT(0); // shoud not happened > + my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); > + return false; > + } I'd still think it'd be better to do a series of asserts. a complex condition to check for something that cannot ever happen? > if (flags & NO_DEFAULT_VALUE_FLAG && > real_type() != MYSQL_TYPE_ENUM) > { > @@ -11221,13 +11253,30 @@ bool Field::save_in_field_default_value(bool view_error_processing) > > bool Field::save_in_field_ignore_value(bool view_error_processing) > { > - enum_sql_command com= table->in_use->lex->sql_command; > - // All insert-like commands > - if (com == SQLCOM_INSERT || com == SQLCOM_REPLACE || > - com == SQLCOM_INSERT_SELECT || com == SQLCOM_REPLACE_SELECT || > - com == SQLCOM_LOAD) > - return save_in_field_default_value(view_error_processing); > - return 0; // ignore > + if ( // table is not opened properly > + table == NULL || table->pos_in_table_list == NULL || > + table->pos_in_table_list->select_lex == NULL || > + // we are in subquery/derived/CTE > + !table->pos_in_table_list->top_table()->select_lex->is_top_level_select()) > + { > + DBUG_ASSERT(0); // shoud not happened > + my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); > + return false; > + } ditto > + switch (find_ignore_reaction(table->in_use)) > + { > + case IGNORE_MEANS_DEFAULT: > + return save_in_field_default_value(view_error_processing); > + case IGNORE_MEANS_FIELD_VALUE: > + return 0; // ignore > + default: > + ; // fall through to error > + } > + > + // unexpected command > + DBUG_ASSERT(0); // shoud not happened > + my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); > + return false; > } > > > diff --git a/sql/item.cc b/sql/item.cc > index 994d45a9dc3..401ee5d6be2 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -84,6 +85,17 @@ void item_init(void) > } > > > +void Item::raise_error_not_evaluable() > +{ > + String tmp; > + this->print(&tmp, QT_ORDINARY); > + // TODO-10.5: add an error message to errmsg-utf8.txt > + my_printf_error(ER_UNKNOWN_ERROR, > + "'%s' is not allowed in this context", MYF(0), tmp.ptr()); > + DBUG_ASSERT(0); // cannot be happen before 10.5 > +} > + > + better remove it, this dead code won't simplify merges. In 10.3 it'll be added with a merge normally. having check_is_evaluable_expression_or_error() makes sense though, it is used and copying it from 10.3 will help to merge indeed or, may be, just backport the patch from 10.3 to 10.2? > void Item::push_note_converted_to_negative_complement(THD *thd) > { > push_warning(thd, Sql_condition::WARN_LEVEL_NOTE, ER_UNKNOWN_ERROR, > @@ -8968,18 +9084,26 @@ bool Item_default_value::fix_fields(THD *thd, Item **items) > def_field->move_field_offset((my_ptrdiff_t) > (def_field->table->s->default_values - > def_field->table->record[0])); > - set_field(def_field); > - return FALSE; > + return def_field; > +} > > -error: > - context->process_error(thd); > - return TRUE; > +bool Item_default_value::associate_with_target_field(THD *thd, > + Item_field *field) > +{ > + associated= true; > + arg= field; iirc, `arg` used to tell the difference between DEFAULT and DEFAULT(x). how are you doing it now? > + return tie_field(thd); > } > > void Item_default_value::cleanup() > { > delete cached_field; // Free cached blob data > cached_field= 0; > + if (associated) > + { > + arg= NULL; > + associated= false; > + } > Item_field::cleanup(); > } > > @@ -9102,8 +9226,54 @@ void Item_ignore_value::print(String *str, enum_query_type query_type) > str->append(STRING_WITH_LEN("ignore")); > } > > + > +bool Item_ignore_value::associate_with_target_field(THD *thd, > + Item_field *field) > +{ > + bitmap_set_bit(field->field->table->read_set, field->field->field_index); > + return Item_default_value::associate_with_target_field(thd, field); > +} > + > + > int Item_ignore_value::save_in_field(Field *field_arg, bool no_conversions) > { > + if (arg) how is IGNORE with arg!=0 differs from IGNORE with arg==0? when happens what? > + { > + switch (find_ignore_reaction(field->table->in_use)) > + { > + case IGNORE_MEANS_DEFAULT: > + DBUG_ASSERT(0); // impossible now, but fully working code if needed > + /* > + save default value > + Note: (((Field*)(arg()->real_item()) is Item_field* (checked in > + tie_field(). > + */ > + if ((((Item_field*)(arg->real_item()))->field->flags & > + NO_DEFAULT_VALUE_FLAG)) > + { > + my_error(ER_NO_DEFAULT_FOR_FIELD, MYF(0), > + ((Item_field*)(arg->real_item()))->field->field_name); > + return true; > + } > + calculate(); > + return Item_field::save_in_field(field_arg, no_conversions); > + case IGNORE_MEANS_FIELD_VALUE: > + // checked on tie_field > + DBUG_ASSERT(arg->real_item()->type() == FIELD_ITEM); > + /* > + save original field (Item::save_in_field is not applicable because > + result_field for the referenced field set to this temporary table). > + */ > + arg->save_val(field_arg); > + return false; > + default: > + ; // fall through to error > + } > + DBUG_ASSERT(0); //impossible > + my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); > + return true; > + } > + > return field_arg->save_in_field_ignore_value(context->error_processor == > &view_error_processor); > } > diff --git a/sql/sp_head.cc b/sql/sp_head.cc > index 6a650183fb8..41a27a3aaaf 100644 > --- a/sql/sp_head.cc > +++ b/sql/sp_head.cc > @@ -418,6 +418,9 @@ sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr) > if (!(expr_item= sp_prepare_func_item(thd, expr_item_ptr))) > goto error; > > + if (expr_item->check_is_evaluable_expression_or_error()) > + goto error; for CALL(?) ? > + > /* > Set THD flags to emit warnings/errors in case of overflow/type errors > during saving the item into the field. > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index cc77b58cb3e..b52d6f96a84 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -8345,6 +8346,9 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values, > field->field_name, table->s->table_name.str); > } > > + if(evaluable && value->check_is_evaluable_expression_or_error()) > + goto err; Is it a good idea to do this check constantly for every user and for every row in selects, insert/update, etc? Could it be done inside IGNORE/DEFAULT? For example, by checking if field->table->tmp_table == INTERNAL_TMP_TABLE. > + > if (use_value) > value->save_val(field); > else > diff --git a/sql/unireg.cc b/sql/unireg.cc > index 083960523c1..91f9073c7ef 100644 > --- a/sql/unireg.cc > +++ b/sql/unireg.cc > @@ -1002,6 +1002,12 @@ static bool make_empty_rec(THD *thd, uchar *buff, uint table_options, > > int res= !expr->fixed && // may be already fixed if ALTER TABLE > expr->fix_fields(thd, &expr); > + if (!res && expr->check_is_evaluable_expression_or_error()) is this for CREATE TABLE case? > + { > + error= 1; > + delete regfield; //To avoid memory leak > + goto err; > + } > if (!res) > res= expr->save_in_field(regfield, 1); > if (!res && (field->flags & BLOB_FLAG)) Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 56f2b3d12b6: MDEV-23264 Unique blobs allow duplicate values upon UPDATE
by Sergei Golubchik 08 Oct '20

08 Oct '20

Hi, Sachin! I suspect is_null() could be wrong here. Please, add a test case where a table has a BEFORE UPDATE trigger and a long UNIQUE over a NOT NULL field. And see how cmp_binary_offset() is used in compare_record(), for example. On Oct 08, Sachin Setiya wrote: > revision-id: 56f2b3d12b6 (mariadb-10.4.11-385-g56f2b3d12b6) > parent(s): af834c218a5 > author: Sachin Setiya <sachin.setiya(a)mariadb.com> > committer: Sachin Setiya <sachin.setiya(a)mariadb.com> > timestamp: 2020-09-14 20:08:18 +0100 > message: > > MDEV-23264 Unique blobs allow duplicate values upon UPDATE > > Problem:- > We are able to insert duplicate value in table because cmp_binary_offset > is not able to differentiate between NULL and empty string. So > check_duplicate_long_entry_key is never called and we don't check for > duplicate. > Solution > Added a if condition with is_null() on field which can differentiate > between NULL and empty string. > > diff --git a/sql/handler.cc b/sql/handler.cc > index 40dea349272..71e6a904f23 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -6699,8 +6699,13 @@ static int check_duplicate_long_entries_update(TABLE *table, handler *h, uchar * > for (uint j= 0; j < key_parts; j++, keypart++) > { > field= keypart->field; > - /* Compare fields if they are different then check for duplicates*/ > - if(field->cmp_binary_offset(reclength)) > + /* > + Compare fields if they are different then check for duplicates > + cmp_binary_offset cannot differentiate between null and empty string > + So also check for that too > + */ > + if((field->is_null(0) != field->is_null(reclength)) || > + field->cmp_binary_offset(reclength)) > { > if((error= check_duplicate_long_entry_key(table, table->update_handler, > new_rec, i))) > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0