October 2017 - developers - lists.mariadb.org

[Maria-developers] MDEV-13997 Change Item_bool_rowready_func2 to cache const items at fix time rather than evaluation time
by Alexander Barkov 29 Oct '17

29 Oct '17

Hello Sergei, Please review a patch for MDEV-13997. It's needed for: MDEV-13995 MAX(timestamp) returns a wrong result near DST change and for: MDEV-4912 Add a plugin to field types (column types) Thanks!

2 3

[Maria-developers] Review Custom Aggregate Functions
by Vicențiu Ciorbaru 28 Oct '17

28 Oct '17

Hi Varun! Here's a final review for Custom Aggregate Functions * You haven't run the full test suite, there are a few failing tests. Please update those. rpl.rpl_sp funcs_1.is_columns_mysql funcs_1.storedproc * Push your code to a BB branch and rebase it on top of current 10.3. * Fix commits messages while rebasing. Use the reword flag to clean those up. The rule is this: 1. Have one line that doesn't exceed 80 Characters as a Title. 2. Leave one empty line 3. Add the rest of the commit message. Don't exceed 80 Characters per line. > diff --git a/sql/sp_head.h b/sql/sp_head.h > index bb516187a57..d447807bb3b 100644 > --- a/sql/sp_head.h > +++ b/sql/sp_head.h > @@ -334,6 +334,11 @@ class sp_head :private Query_arena, > execute_function(THD *thd, Item **args, uint argcount, Field *return_fld); > > bool Rename this to execute_function please and delete the now dead-code from the current execute_function. Update comments for this function appropriately. Please run all tests again after this and make sure you update results for all the failing ones. > + execute_aggregate_function(THD *thd, Item **args, uint argcount, > + Field *return_fld, sp_rcontext **nctx, > + MEM_ROOT *caller_mem_root); > + > + bool > execute_procedure(THD *thd, List<Item> *args); > > static void > @@ -813,6 +818,9 @@ class sp_head :private Query_arena, > bool > execute(THD *thd, bool merge_da_on_success); > This is no longer used please remove! > + bool > + execute_agg(THD *thd, bool merge_da_on_success); > + > /** > Perform a forward flow analysis in the generated code. > Mark reachable instructions, for the optimizer. > +++ b/mysql-test/t/sp_agg.test A lot of empty lines at the end of this test. Please delete those. Ping me once you have a green buildbot before pushing. Vicentiu

1 0

[Maria-developers] Review of Percentile Window Functions
by Vicențiu Ciorbaru 28 Oct '17

28 Oct '17

Hi Varun, Here's the final review for Percentile Functions. Just 2 minor coding style comments. Rebase on top of current 10.3 and push to a bb-* branch to double check nothing breaks. Make sure that no merge commit happens. Regards, Vicențiu > diff --git a/sql/item_windowfunc.h b/sql/item_windowfunc.h > index 77cbd556e60..589f7cf5d96 100644 > --- a/sql/item_windowfunc.h > +++ b/sql/item_windowfunc.h > @@ -678,6 +687,277 @@ class Item_sum_ntile : public Item_sum_window_with_row_count > ulong current_row_count_; > }; > > +class Item_sum_percentile_disc : public Item_sum_cume_dist, > + public Type_handler_hybrid_field_type > +{ > +public: > + Item_sum_percentile_disc(THD *thd, Item* arg) : Item_sum_cume_dist(thd, arg), > + Type_handler_hybrid_field_type(&type_handler_longlong), > + value(NULL), val_calculated(FALSE), first_call(TRUE), > + prev_value(0), order_item(NULL){} > + > + double val_real() > + { > + if (get_row_count() == 0 || get_arg(0)->is_null()) > + { > + null_value= true; > + return 0; > + } > + null_value= false; > + return value->val_real(); > + } > + > + longlong val_int() > + { > + if (get_row_count() == 0 || get_arg(0)->is_null()) > + { > + null_value= true; > + return 0; > + } > + null_value= false; > + return value->val_int(); > + } > + > + my_decimal* val_decimal(my_decimal* dec) > + { > + if (get_row_count() == 0 || get_arg(0)->is_null()) > + { > + null_value= true; > + return 0; > + } > + null_value= false; > + return value->val_decimal(dec); > + } > + > + String* val_str(String *str) > + { > + if (get_row_count() == 0 || get_arg(0)->is_null()) > + { > + null_value= true; > + return 0; > + } > + null_value= false; > + return value->val_str(str); > + } > + > + bool add() > + { > + Item *arg= get_arg(0); > + if (arg->is_null()) > + return false; > + > + if (first_call) > + { > + prev_value= arg->val_real(); > + if (prev_value > 1 || prev_value < 0) > + { > + my_error(ER_ARGUMENT_OUT_OF_RANGE, MYF(0), func_name()); > + return true; > + } > + first_call= false; > + } > + > + double arg_val= arg->val_real(); > + > + if (prev_value != arg_val) > + { > + my_error(ER_ARGUMENT_NOT_CONSTANT, MYF(0), func_name()); > + return true; > + } > + > + if (val_calculated) > + return false; > + > + value->store(order_item); > + value->cache_value(); > + if (value->null_value) > + return false; > + > + Item_sum_cume_dist::add(); > + double val= Item_sum_cume_dist::val_real(); > + > + if (val >= prev_value && !val_calculated) > + val_calculated= true; > + return false; > + } > + > + enum Sumfunctype sum_func() const > + { > + return PERCENTILE_DISC_FUNC; > + } > + > + void clear() > + { > + val_calculated= false; > + first_call= true; > + value->clear(); > + Item_sum_cume_dist::clear(); > + } > + > + const char*func_name() const > + { > + return "percentile_disc"; > + } > + > + void update_field() {} > + void set_type_handler(Window_spec *window_spec); > + const Type_handler *type_handler() const > + {return Type_handler_hybrid_field_type::type_handler();} > + > + void fix_length_and_dec() > + { > + decimals = 10; // TODO-cvicentiu find out how many decimals the standard > + // requires. > + } > + > + Item *get_copy(THD *thd, MEM_ROOT *mem_root) > + { return get_item_copy<Item_sum_percentile_disc>(thd, mem_root, this); } > + void setup_window_func(THD *thd, Window_spec *window_spec); > + void setup_hybrid(THD *thd, Item *item); > + bool fix_fields(THD *thd, Item **ref); > + > +private: > + Item_cache *value; > + bool val_calculated; > + bool first_call; > + double prev_value; > + Item *order_item; > +}; > + > +class Item_sum_percentile_cont : public Item_sum_cume_dist, > + public Type_handler_hybrid_field_type > +{ > +public: > + Item_sum_percentile_cont(THD *thd, Item* arg) : Item_sum_cume_dist(thd, arg), > + Type_handler_hybrid_field_type(&type_handler_double), > + floor_value(NULL), ceil_value(NULL), first_call(TRUE),prev_value(0), > + ceil_val_calculated(FALSE), floor_val_calculated(FALSE), order_item(NULL){} > + > + double val_real() > + { > + if (get_row_count() == 0 || get_arg(0)->is_null()) > + { > + null_value= true; > + return 0; > + } > + null_value= false; > + double val= 1 + prev_value * (get_row_count()-1); > + > + /* > + Applying the formula to get the value > + If (CRN = FRN = RN) then the result is (value of expression from row at RN) > + Otherwise the result is > + (CRN - RN) * (value of expression for row at FRN) + > + (RN - FRN) * (value of expression for row at CRN) > + */ > + > + if(ceil(val) == floor(val)) > + return floor_value->val_real(); > + > + double ret_val= ((val - floor(val)) * ceil_value->val_real()) + > + ((ceil(val) - val) * floor_value->val_real()); > + > + return ret_val; > + } > + > + bool add() > + { > + Item *arg= get_arg(0); > + if (arg->is_null()) > + return false; > + > + if (first_call) > + { > + first_call= false; > + prev_value= arg->val_real(); > + if (prev_value > 1 || prev_value < 0) > + { > + my_error(ER_ARGUMENT_OUT_OF_RANGE, MYF(0), func_name()); > + return true; > + } > + } > + > + double arg_val= arg->val_real(); > + if (prev_value != arg_val) > + { > + my_error(ER_ARGUMENT_NOT_CONSTANT, MYF(0), func_name()); > + return true; > + } > + > + if (!floor_val_calculated) > + { > + floor_value->store(order_item); > + floor_value->cache_value(); > + if (floor_value->null_value) > + return false; Add one extra space before return > + } > + if (floor_val_calculated && !ceil_val_calculated) > + { > + ceil_value->store(order_item); > + ceil_value->cache_value(); > + if (ceil_value->null_value) > + return false; Add one extra space before return > + } > + > + Item_sum_cume_dist::add(); > + double val= 1 + prev_value * (get_row_count()-1); > + > + if (!floor_val_calculated && get_row_number() == floor(val)) > + floor_val_calculated= true; too much indentation here. Delete 2 spaces > + > + if (!ceil_val_calculated && get_row_number() == ceil(val)) > + ceil_val_calculated= true; too much indentation here. Delete 2 spaces > + return false; > + } > + > + enum Sumfunctype sum_func() const > + { > + return PERCENTILE_CONT_FUNC; > + } > + > + void clear() > + { > + first_call= true; > + floor_value->clear(); > + ceil_value->clear(); > + floor_val_calculated= false; > + ceil_val_calculated= false; > + Item_sum_cume_dist::clear(); > + } > + > + const char*func_name() const > + { > + return "percentile_cont"; > + } > + void update_field() {} > + void set_type_handler(Window_spec *window_spec); > + const Type_handler *type_handler() const > + {return Type_handler_hybrid_field_type::type_handler();} > + > + void fix_length_and_dec() > + { > + decimals = 10; // TODO-cvicentiu find out how many decimals the standard > + // requires. > + } > + > + Item *get_copy(THD *thd, MEM_ROOT *mem_root) > + { return get_item_copy<Item_sum_percentile_cont>(thd, mem_root, this); } > + void setup_window_func(THD *thd, Window_spec *window_spec); > + void setup_hybrid(THD *thd, Item *item); > + bool fix_fields(THD *thd, Item **ref); > + > +private: > + Item_cache *floor_value; > + Item_cache *ceil_value; > + bool first_call; > + double prev_value; > + bool ceil_val_calculated; > + bool floor_val_calculated; > + Item *order_item; > +}; > + > + > + > > class Item_window_func : public Item_func_or_sum > { > diff --git a/sql/lex.h b/sql/lex.h > index 17dd45f9c0b..fcb44a0f836 100644 > --- a/sql/lex.h > +++ b/sql/lex.h > @@ -735,6 +736,7 @@ static SYMBOL sql_functions[] = { > { "LAG", SYM(LAG_SYM)}, > { "LEAD", SYM(LEAD_SYM)}, > { "MAX", SYM(MAX_SYM)}, > + { "MEDIAN", SYM(MEDIAN_SYM)}, Fix indentation here > { "MID", SYM(SUBSTRING)}, /* unireg function */ > { "MIN", SYM(MIN_SYM)}, > { "NOW", SYM(NOW_SYM)}, > \ No newline at end of file

1 0

Re: [Maria-developers] 9145b7d: MDEV-11153 - Introduce status variables for table cache monitoring and tuning
by Sergei Golubchik 27 Oct '17

27 Oct '17

Hi, Sergey! Looks ok, thanks. Just one question: On Aug 10, Sergey Vojtovich wrote: > revision-id: 9145b7dd11a0e95016304e53acf306daac323580 (mariadb-10.3.0-49-g9145b7d) > parent(s): 63ad4fe5bbe77fe17ab441fcce0106bdee9b618d > committer: Sergey Vojtovich > timestamp: 2017-08-10 15:45:03 +0400 > message: > > MDEV-11153 - Introduce status variables for table cache monitoring and tuning > > Status variables added: Table_open_cache_hits, Table_open_cache_misses, > Table_open_cache_overflows. > > System variables added: table_open_cache_active_instances (read-only). > > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index e89dd47..92721b6 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -3219,6 +3219,12 @@ static Sys_var_ulong Sys_table_cache_size( > BLOCK_SIZE(1), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0), > ON_UPDATE(fix_table_open_cache)); > > +static Sys_var_uint Sys_table_cache_active_instances( > + "table_open_cache_active_instances", > + "Number of active table cache instances", > + READ_ONLY GLOBAL_VAR(tc_active_instances), NO_CMD_LINE, > + VALID_RANGE(1, 64), DEFAULT(1), BLOCK_SIZE(1)); Why did you do it as a sysvar, not as a status variable? Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

[Maria-developers] Code syntax: questions on pointers, etc.
by Aleksey Midenkov 27 Oct '17

27 Oct '17

Recent refactorings of replacing C strings with LEX_CSTRING which is no doubt a good thing raise some questions: 1. Is it still guaranteed that Field::field_name.str is NULL-terminated? 2. It is still passed as a pointer to functions. Why is that? The main feature of C++ references is that it cannot be NULL, so we get segfault on top of the stack (closer to a cause), not the bottom of it. I see that pointers are now widely used and mainly assumed to be always non-NULL (i.e. dereferenced without assertion). But placing such implicit contract on data is not evident and bug-prone. IMHO it's much better to use references whenever it is possible (and when there is no need in cosy NULL semantic). What do you think? But for such lightweight structs like LEX_CSTRING it is even better to pass by value, so we could have the conventience of type cast. 3. LEX_CSTRING and LEX_STRING are now non-convertible. Why not to make: template <typename char_t> struct st_mysql_lex_string { char_t *str; size_t length; }; typedef st_mysql_lex_string<char *> LEX_STRING; typedef st_mysql_lex_string<const char *> LEX_CSTRING; typedef st_mysql_lex_string<const unsigned char *> LEX_CUSTRING; ? 4. There are some duplicate types: MYSQL_LEX_STRING, MYSQL_CONST_LEX_STRING. Why? -- All the best, Aleksey Midenkov @midenok

2 3

[Maria-developers] Obsolete GTID domain delete on master (MDEV-12012, MDEV-11969)
by andrei.elkin＠pp.inet.fi 24 Oct '17

24 Oct '17

Hello. Let me propose methods to clean master off unused gtid domains. The issue is quite practical, as a couple of references on the subject line tell. Either of them narrates a scenario of two default domain-id masters serving to one slave initially by "legacy" non-gtid protocol. When later the masters have changed their common domain to different private ones *and* the slave turns gtid replication ON, it can't connect to a master due to the gtid protocol. And the reason is the prior-to-gtid-connect gained slave's gtid position consisting of gtids generated by the other master obviously (the two masters never replicated each other) does not fit to the current master binlog state. The past default domain-id is actually permanent past from the user perspective in these cases. Its events have been already replicated and none new will be generated and replicated. Therefore such domain conceptually may be cleaned away from either the masters and slave states. Once it's done, the gtid-enabled slave will successfully connect to any master. The slave state purge is simple SET @@global.gtid_slave_pos to a sequence free of the purged domain. The master side binlog state one requires a new "command" unless the user is happy with RESET MASTER. While setting the new gtid binlog state to be old-domain-free we would like for the new "command" to preserve the existing binlog files. This could be accomplished as Kristian suggests in MDEV-12012 (I could not find any earlier references to the idea) as a new option to FLUSH BINARY LOGS DELETE DOMAIN d1, d2 KN> This command would check that the current binlog state is equal to KN> the GTID_LIST_LOG_EVENT at the start of the first binary log file, KN> within the specified domains. If not, it is an error. But if so, KN> the new binary log file created by FLUSH will be written with the KN> specified domains omitted from GTID_LIST_LOG_EVENT (and the current KN> binlog state updated accordingly). The idea looks quite sane, I only could not grasp why presence of being deleted domains in the very first binlog's GTID_LIST_LOG_EVENT list is warrant for throwing error. Maybe we should leave it out to the user, Kristian? That is to decide what domain is garbage regardless of the binlog state history. While the FLUSH way looks sufficient and robust I could not help to think over an alternative. Consider a scenario when a domain's sequence number got run out of range. While deems unrealistic in practice we can simulate it with SET @@SESSION.gtid_domain_id=11; SET @@SESSION.server_id=1; SET @@SESSION.gtid_seq_no=18446744073709551615; /* Exec some dummy loggable query, e.g */ CREATE TABLE IF NOT EXISTS `table_dummy`; SHOW LOCAL VARIABLES LIKE '%gtid_binlog_pos%'; 11-1-18446744073709551615 SET @@SESSION.gtid_seq_no=0 DROP TABLE `table_dummy`; SHOW LOCAL VARIABLES LIKE '%gtid_binlog_pos%'; 11-1-0 I've used two gtids to show domain overflow because I also liked to read the zero of the last gtid as ... there's *nothing* in this domain. So it's actually a new "namesake" one, replacing the old that is wrapped around. The 1st group of events created in the new domain - 11-1-1 - could shadow the old domain's 11-1-1 as well as all the rest of the old domain from gtid replication protocol. And that means the old domain is actually deleted. So if my reading of zero is correct the binlog status would be empty instead. That's how we also can approach the master side old gtid domain purging: 1. Leave wrapping around an old domain to the user via running the queries like above; 2. The binary logger would be made to react on the fact of wrap-around with binary log rotation ("internal" FLUSH BINARY LOG). And the new binlog file won't contain the wrapped "away" domain (because there are no new event group in it of yet). I would be glad to hear your opinions, dear colleagues. Cheers, Andrei

4 27

[Maria-developers] Bitmap indexing with Mariadb
by chanaka rathnayaka 16 Oct '17

16 Oct '17

Hi, I am Chanaka who is an undergraduate engineering student. I am doing a research to check, how bitmap indexing improves the query performance and I hope to use MariaDB as database engine. So I need to know whether MariaDB has bitmap indexing already or not ? Thank you, Regards, Chanaka <http://www.justforlearning.info> Chanaka Rathnayaka / undergraduate, Department of Computer Science and Engineering, University of Moratuwa, Sri Lanka bmcrathnayaka(a)gmail.com / +94776608457 [image: Twitter] <https://htmlsig.com/t/0000001BX4MCF> [image: Facebook] <https://htmlsig.com/t/0000001BR3W18> [image: LinkedIn] <https://htmlsig.com/t/0000001BT4W90> [image: Github] <https://htmlsig.com/t/0000001BYB6WK>

2 1

[Maria-developers] MDEV-13550 is test case required?
by Aleksey Midenkov 14 Oct '17

14 Oct '17

Hello! Do we have to write test cases for such coding errors? Because this is developer factor: either he does such errors or not. What are conditions of getting PR merged?

2 2

Re: [Maria-developers] MDEV-14013
by Alexander Barkov 14 Oct '17

14 Oct '17

Hello Jerome, Thank you very much for you contribution! On 10/11/2017 02:32 PM, jerome brauge wrote: > Hello Alexander, > Here is the patch for MDEV-14013, can you review it ? The patch looks very good. I only found small things: 1. LEX::make_text_literal_concat() thd->variables.character_set_client was used instread of thd->variables.collation_connection 2. LEX::make_text_literal_concat() was somewhat hard to read. I propose to change the logic to do: if (item->type() == Item::NULL_ITEM) { } instead of if (item->type() != Item::NULL_ITEM || !(thd->variables.sql_mode & MODE_EMPTY_STRING_IS_NULL)) I think it looks clearer this way. Also, I added some DBUG_ASSERTs to make more sure that a non-Item_string item is not erroneously cast to Item_string. 3. Also, I found that it's easier to place the new methods into THD instead of LEX. Please find a patch with changes 1,2,3 attached. Do you agree with these changes? Also, can you please add some tests: - SHOW CREATE VIEW into "Test in a view" in empty_string_literal.result.diff - EXPLAIN EXTENDED for the affected grammar: EXPLAIN EXTENDED SELECT ''; EXPLAIN EXTENDED SELECT _latin1''; EXPLAIN EXTENDED SELECT N''; EXPLAIN EXTENDED SELECT '' ''; Thanks! > > Thanks. > > >> -----Message d'origine----- >> De : Alexander Barkov [mailto:bar@mariadb.org] >> Envoyé : mercredi 11 octobre 2017 04:15 >> À : jerome brauge >> Objet : Re: Question about MDEV-10574 >> >> Hello Jerome, >> >> >> On 10/10/2017 04:23 PM, jerome brauge wrote: >>> Hello Alexander, >>> For sql_mode EMPTY_STRING_IS_NULL, do you think that it must cover call >> of statement with use of user variable initialized with an empty string ? >>> >>> Ex : >>> SET sql_mode=default; >>> SET @param=''; >>> >>> SET sql_mode=EMPTY_STRING_IS_NULL; >>> PREPARE test FROM 'select 1,?'; >>> EXECUTE test USING @param; >>> >>> Or >>> >>> delimiter / >>> CREATE OR REPLACE PROCEDURE p1(IN a VARCHAR(50)) BEGIN >>> IF a is null THEN >>> SELECT 'IS NULL'; >>> ELSE >>> select 'IS NOT NULL'; >>> END IF; >>> END; >>> / >>> delimiter ; >>> SET sql_mode=default; >>> SET @a=''; >>> >>> SET sql_mode=EMPTY_STRING_IS_NULL; >>> EXECUTE IMMEDIATE 'CALL p1(?)' USING @a; CALL p1(@a); >>> >>> For me, it's a study case and we can forget it. >> >> I agree. Oracle doesn't have user variables. >> >>> >>> Regards, >>> Jérôme. >>> >>>> -----Message d'origine----- >>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : samedi 7 >>>> octobre 2017 11:44 À : jerome brauge Objet : Re: Patch for MDEV-10574 >>>> >>>> Hello Jerome, >>>> >>>> >>>> On 10/06/2017 01:41 PM, jerome brauge wrote: >>>>> Hello Alexander, >>>>> >>>>>> -----Message d'origine----- >>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : jeudi 5 >>>>>> octobre 2017 21:26 À : jerome brauge Objet : Re: Patch for >>>>>> MDEV-10574 >>>>>> >>>>>> Hello Jerome, >>>>>> >>>>>> >>>>>> On 10/05/2017 12:45 PM, jerome brauge wrote: >>>>>>> Hello Alexander, >>>>>>> Have you gone ahead on your reflection about nulls and empty strings >> ? >>>>>> >>>>>> Yes, I think so. >>>>>> >>>>>> One addition: Monty suggested to rename flags to: >>>>>> - EMPTY_STRING_IS_NULL for literals >>>>>> - EMPTY_STRING_IS_NULL_RESULTS for functions. >>>>>> >>>>>> I'm fine with EMPTY_STRING_IS_NULL for literals, but think that >>>>>> EMPTY_STRING_IS_NULL_FUNCTIONS should be more clear than >>>>>> EMPTY_STRING_IS_NULL_RESULTS. >>>>>> Which name do you prefer for functions? >>>>> >>>>> I think like you. >>>>> For me, EMPTY_STRING_IS_NULL_RESULTS sounds like including >>>> RESULTSET >>>>> of select (which is perhaps included in point 4) >>>> >>>> Okey, let's go with _FUNCTIONS then. >>>> >>>>> >>>>>>> >>>>>>> Can I >>>>>>> - redo my pull request for substring_oracle function >>>>>>> - make a patch for first new sql_mode >>>>>> (MODE_EMPTY_AS_NULL_LITERALS) >>>>>> >>>>>> >>>>>> Yes, please. Let's move forward. >>>>>> >>>>>> Let's push the patch for substr_oracle first. >>>>>> Let's keep it return empty strings for the cases when the >>>>>> substring_length parameters is less than 1. >>>>>> It will start automatically returning NULL when we add the new >>>>>> sql_mode for functions. >>>>> >>>>> Pull request is done. >>>> >>>> I have merged it. Thanks! >>>> >>>>> >>>>>> Also, let's go ahead with the part for literals. >>>>> >>>>> Ok, I will make a separate patch. >>>>> Many thanks. >>>>> >>>>>> >>>>>> >>>>>> I have created MDEVs. Please use them in the commit comment and in >>>>>> the >>>>>> tests: >>>>>> >>>>>> MDEV-14012 sql_mode=Oracle: substr(): treat position 0 as position >>>>>> 1 >>>>>> MDEV-14013 sql_mode=EMPTY_STRING_IS_NULL >>>>>> >>>>>> >>>>>> Thanks! >>>>>> >>>>>>> >>>>>>> Regards, >>>>>>> Jérôme. >>>>>>> >>>>>>> >>>>>>>> -----Message d'origine----- >>>>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mercredi >>>>>>>> 27 septembre 2017 08:50 À : jerome brauge Objet : Re: Patch for >>>>>>>> MDEV-10574 >>>>>>>> >>>>>>>> Hello Jerome, >>>>>>>> >>>>>>>> I told to Monty yesterday. >>>>>>>> >>>>>>>> We were going to implement the following transformations: >>>>>>>> >>>>>>>>> We are considering to implement both of the following options >>>>>>>>> (with a >>>>>>>>> switch) >>>>>>>>> >>>>>>>>> Transform Insert >>>>>>>>> - insert values ("") -> insert values (null) >>>>>>>>> >>>>>>>>> Transform Select >>>>>>>>> >>>>>>>>> where v=x => (v <> "" and V=X) >>>>>>>>> where v is null => (v="" or v is null) >>>>>>>>> >>>>>>>> >>>>>>>> See MDEV-10574 for details. >>>>>>>> >>>>>>>> Monty did not fully understand the idea of translating literals >>>>>>>> and >>>>>> functions >>>>>>>> from empty strings to null. He asked if you can explain use cases >>>>>>>> for these transformations. >>>>>>>> >>>>>>>> But we agreed that: >>>>>>>> >>>>>>>> 1. It's OK to have multiple NULL handling flags in sql_mode. >>>>>>>> >>>>>>>> 2. New flags will not be enabled in 10.3, neither in >>>>>>>> sql_mode=DEFAULT, >>>>>> nor in >>>>>>>> sql_mode=ORACLE. So one will have to specify them explicitly: >>>>>>>> set >>>>>>>> >>>>>> >>>> >> sql_mode='ORACLE,MODE_null_transfrormation_flag1,MODE_null_transfor >>>>>>>> mation_flag2'; >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> So I propose we introduce the following flags: >>>>>>>> >>>>>>>> >>>>>>>> 1. MODE_EMPTY_AS_NULL_LITERALS - for literals and PS >> parameters, >>>> as >>>>>> in >>>>>>>> your patch. >>>>>>>> >>>>>>>> 2. MODE_EMPTY_AS_NULL_FUNCTIONS - for functions. >>>>>>>> This should be done as a separate patch. >>>>>>>> Note, your patch currently implements empty-to-null translation >>>>>>>> only for Item_str_func descendants that use >> make_empty_result(). >>>>>>>> This patch should be extended to cover Item_str_func descendants >>>>>>>> which do not use make_empty_result(), as well as >>>>>>>> all other Item_func_or_sum descendants >>>>>>>> (which are not Item_str_func descendants). >>>>>>>> >>>>>>>> >>>>>>>> 3. MODE_EMPTY_AS_NULL_UPDATES >>>>>>>> >>>>>>>> For INSERT and UPDATE transformation, as in MDEV: >>>>>>>> >>>>>>>> insert values ("") -> insert values (null) >>>>>>>> update set a='' -> update set a=null; >>>>>>>> >>>>>>>> >>>>>>>> 4. MODE_EMPTY_AS_NULL_PREDICATES >>>>>>>> >>>>>>>> For predicate transformation, as in MDEV: >>>>>>>> >>>>>>>> where v=x => (v <> "" and V=X) >>>>>>>> where v is null => (v="" or v is null) >>>>>>>> >>>>>>>> >>>>>>>> What do you think about this proposal with multiple flags? >>>>>>>> Note, we can do N1 and N2 now, and add N3 and N4 and later. >>>>>>>> >>>>>>>> Also, can you please write a description why you need N1 and N2. >>>>>>>> Monty thinks that N3 and N4 should cover most use cases. >>>>>>>> So we need a good rationale for Monty and Sergei explaining why >>>>>>>> we >>>>>> adding >>>>>>>> these flags N1 and N2. >>>>>>>> >>>>>>>> >>>>>>>> Thanks! >>>>>>>> >>>>>>>> >>>>>>>> On 09/26/2017 04:33 PM, jerome brauge wrote: >>>>>>>>> Hi Alexander, >>>>>>>>> I'm disappointed. >>>>>>>>> Can we at least keep this part in sql_prepare : >>>>>>>>> >>>>>>>>> /* >>>>>>>>> set_param_str_oracle : convert empty string to null value >>>>>>>>> This allow to bind empty string in JDBC as a null value. >>>>>>>>> Ex : >>>>>>>>> String sel="select coalesce(?,'null param') from dual"; >>>>>>>>> PreparedStatement ps_sel = m_cnx.prepareStatement(sel); >>>>>>>>> ps_sel.setString(1, ""); >>>>>>>>> ResultSet rs = ps_sel.executeQuery(); >>>>>>>>> while (rs.next()) >>>>>>>>> { >>>>>>>>> System.out.println(rs.getString(1)); >>>>>>>>> } >>>>>>>>> Returns : 'null param' >>>>>>>>> */ >>>>>>>>> static void set_param_str_oracle(Item_param *param, uchar >> **pos, >>>>>> ulong >>>>>>>>> len) { >>>>>>>>> ulong length= get_param_length(pos, len); >>>>>>>>> if (length == 0) >>>>>>>>> param->set_null(); >>>>>>>>> else >>>>>>>>> { >>>>>>>>> if (length > len) >>>>>>>>> length= len; >>>>>>>>> param->set_str((const char *) *pos, length); >>>>>>>>> *pos+= length; >>>>>>>>> } >>>>>>>>> } >>>>>>>>> >>>>>>>>> Regards, >>>>>>>>> Jérôme. >>>>>>>>> >>>>>>>>> >>>>>>>>>> -----Message d'origine----- >>>>>>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mardi >>>>>>>>>> 26 septembre 2017 13:34 À : jerome brauge Objet : Re: Patch for >>>>>>>>>> MDEV-10574 >>>>>>>>>> >>>>>>>>>> Hello Jérôme, >>>>>>>>>> >>>>>>>>>> I'm afraid we cannot accept this change at this point of time. >>>>>>>>>> >>>>>>>>>> The problem is that all around the code we assume that NULL is >>>>>>>>>> not equal to ''. >>>>>>>>>> >>>>>>>>>> Changing '' to NULL only in the visible part of the iceberg >>>>>>>>>> (such as in the parser and in >>>>>>>>>> Item_str_func::make_empty_result()) >>>>>>>>>> will likely introduce various anomalies in fundamental things >>>>>>>>>> like joins execution, optimizer, equality propagation, etc. >>>>>>>>>> >>>>>>>>>> Sorry, we don't have resources to dive into this topic deeper now. >>>>>>>>>> Moreover, Oracle's documentation suggests not to reply on the >>>>>>>>>> fact >>>>>> that '' >>>>>>>>>> and NULL are treated as same, as this can change in the future. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 09/20/2017 03:52 PM, jerome brauge wrote: >>>>>>>>>>> Hello Alexander, >>>>>>>>>>> Here is a new version of this patch (minor optimizations) >>>>>>>>>>> >>>>>>>>>>> Regards, >>>>>>>>>>> Jérôme. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> -----Message d'origine----- >>>>>>>>>>>> De : Maria-developers [mailto:maria-developers- >>>>>>>>>>>> bounces+j.brauge=qualiac.com(a)lists.launchpad.net] De la part >>>> de >>>>>>>>>>>> bounces+jerome >>>>>>>>>>>> brauge >>>>>>>>>>>> Envoyé : mardi 19 septembre 2017 16:09 À : 'Alexander Barkov' >>>>>>>>>>>> Cc : MariaDB Developers >>>>>>>>>>>> (maria-developers(a)lists.launchpad.net) >>>>>>>>>>>> Objet : [Maria-developers] Patch for MDEV-10574 >>>>>>>>>>>> >>>>>>>>>>>> Alexander, >>>>>>>>>>>> Here is the patch I was talking about. >>>>>>>>>>>> It not address all aspects of MDEV-10574, especially, empty >>>>>>>>>>>> strings in table column are not view as null. >>>>>>>>>>>> But as in oracle mode, no empty string in columns should be >>>>>>>>>>>> created, an fully oracle application will never fall in this case. >>>>>>>>>>>> >>>>>>>>>>>> Regards, >>>>>>>>>>>> Jérôme. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> -----Message d'origine----- >>>>>>>>>>>>> De : jerome brauge >>>>>>>>>>>>> Envoyé : mardi 19 septembre 2017 14:13 À : 'Alexander >> Barkov' >>>>>>>>>>>>> Objet : RE: Oracle strings functions compatibility : substr >>>>>>>>>>>>> >>>>>>>>>>>>> Hello Alexander, >>>>>>>>>>>>> Yes I'm aware. >>>>>>>>>>>>> This point will be treated by my next patch which will also >>>>>>>>>>>>> affect the others string functions (a part of MDEV-10574). >>>>>>>>>>>>> I will refresh this new patch with the current development >>>>>>>>>>>>> branch, and I submit to you. >>>>>>>>>>>>> Regards. >>>>>>>>>>>>> Jérôme. >>>>>>>>>>>>> >>>>>>>>>>>>>> -----Message d'origine----- De : Alexander Barkov >>>>>>>>>>>>>> [mailto:bar@mariadb.org] Envoyé : >>>> mardi >>>>>> 19 >>>>>>>>>>>>>> septembre 2017 13:16 À : jerome brauge Objet : Re: Oracle >>>>>>>>>>>>>> strings functions compatibility : substr >>>>>>>>>>>>>> >>>>>>>>>>>>>> Hi Jerome, >>>>>>>>>>>>>> >>>>>>>>>>>>>> I noticed one more difference in SUBSTR: >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> SELECT SUBSTR('aaa',1,-1) FROM DUAL; >>>>>>>>>>>>>> >>>>>>>>>>>>>> MariaDB returns an empty string. >>>>>>>>>>>>>> Oracle returns NULL. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Here's Oracle's documentation: >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>> >>>>>> >> https://docs.oracle.com/cd/B19306_01/server.102/b14200/functions162. >>>>>>>>>>>>>> ht >>>>>>>>>>>>>> m >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> I think we should do this in the same patch, to avoid >>>>>>>>>>>>>> behavior change in the future. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Would you like to try doing this additional change? >>>>>>>>>>>>>> Or would you like me to make an incremental patch on top >> of >>>>>> yours? >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks! >>>>>>>>>>>>>> >>>>>>>>>>>>>> On 09/19/2017 10:33 AM, jerome brauge wrote: >>>>>>>>>>>>>>> Hello Alexander, >>>>>>>>>>>>>>> It's done. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Do you have news from Sergei for MDEV-12874, MDEV- >> 13417 >>>>>> and >>>>>>>>>>>> MDEV- >>>>>>>>>>>>>> 13418 ? >>>>>>>>>>>>>> >>>>>>>>>>>>>> I asked Sergei, awaiting for his answer. I'll let you know. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Thank you very much. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> -----Message d'origine----- De : Alexander Barkov >>>>>>>>>>>>>>>> [mailto:bar@mariadb.org] Envoyé : lundi >>>>>>>>>>>>>>>> 18 septembre 2017 18:12 À : jerome brauge Objet : Re: >>>>>>>>>>>>>>>> Oracle strings functions compatibility : substr >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Hello Jerome, >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On 09/18/2017 05:14 PM, jerome brauge wrote: >>>>>>>>>>>>>>>>> Hello Alexander, >>>>>>>>>>>>>>>>> I don't understand how I've missed such an obvious >>>> solution ! >>>>>>>>>>>>>>>>> Here is the new patch. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Thanks. Now it looks simpler. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Can you please do one small thing: >>>>>>>>>>>>>>>> move get_position() from "public:" to "protected:". >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Ok to make a pull request after this change. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Thank you very much! >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Regards. >>>>>>>>>>>>>>>>> Jérôme. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> -----Message d'origine----- De : Alexander Barkov >>>>>>>>>>>>>>>>>> [mailto:bar@mariadb.org] Envoyé : >>>>>> lundi >>>>>>>>>>>>>>>>>> 18 septembre 2017 11:30 À : jerome brauge Objet : Re: >>>>>> Oracle >>>>>>>>>>>>>>>>>> strings functions compatibility : substr >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Hello Jerome, >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> On 08/17/2017 04:43 PM, jerome brauge wrote: >>>>>>>>>>>>>>>>>>> Hello Alexander, >>>>>>>>>>>>>>>>>>> Here is a patch for function SUBSTR in >> sql_mode=oracle >>>>>>>>>>>>>>>>>>> (If position is 0, >>>>>>>>>>>>>>>>>> then it is treated as 1). >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> I'm thinking of how to get a smaller patch. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Wouldn't it be possible to introduce: >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> virtual longlong get_position(); >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> longlong Item_func_substr::get_position() { >>>>>>>>>>>>>>>>>> return args[1]->val_int(); } >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> longlong Item_func_substr_oracle::get_position() >>>>>>>>>>>>>>>>>> { >>>>>>>>>>>>>>>>>> longlong pos= args[1]->val_int(); >>>>>>>>>>>>>>>>>> return pos == 0 ? 1 : pos; } >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> and just replace all calls for args[1]->val_int() to >>>>>> get_position() >>>>>>>> ? >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Thanks! >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Regards, >>>>>>>>>>>>>>>>>>> Jérôme. >>>>>>>>>>>>>>>>>>>

2 2

Re: [Maria-developers] [Commits] e0a1e63: MDEV-9619: Assertion `null_ref_table' failed in virtual table_map Item_direct_view_ref::used_tables() const on 2nd execution of PS
by Sergei Golubchik 13 Oct '17

13 Oct '17

Hi, Oleksandr! On Feb 18, Oleksandr Byelkin wrote: > revision-id: e0a1e632e4e6084cd989f217943618f4e9ac9179 (mariadb-5.5.54-25-ge0a1e63) > parent(s): 29d78dbb44ee9890b6bc28873344f20fc9157928 > committer: Oleksandr Byelkin > timestamp: 2017-02-18 17:47:31 +0100 > message: > > MDEV-9619: Assertion `null_ref_table' failed in virtual table_map > Item_direct_view_ref::used_tables() const on 2nd execution of PS > > Refer left expression indirectly in case it changes from execution to > execution. Looks reasonable. Ok to push with these changes: 1. replace - DBUG_ASSERT(null_ref_table); + DBUG_ASSERT(fixed); as discussed. 2. Remove bool Item_direct_view_ref::fix_fields(THD *thd, Item **reference) { - DBUG_ASSERT(1); these two changes should, probably, be done in a separate cleanup commit. 3. > diff --git a/sql/item.h b/sql/item.h > index 89155ac..dff7bc4 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -1543,6 +1543,8 @@ class Item { > virtual void mark_as_condition_AND_part(TABLE_LIST *embedding) {}; > }; > > +typedef Item (*Item_ptr); > + remove this, your patch compiles just fine without it. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0