22 Mar
2017
22 Mar
'17
10:37 a.m.
Hello Alexander,
All works fine, thank you very much.
Now, we are blocked by MDEV-12137 (delete or update statement with the same source and target).
When do you think you can work on it?
Regards,
Jérôme.
> -----Message d'origine-----
> De : Alexander Barkov [mailto:bar@mariadb.org]
> Envoyé : jeudi 16 mars 2017 13:03
> À : jerome brauge
> Cc : MariaDB Developers (maria-developers@lists.launchpad.net)
> Objet : Re: MDEV-10598 - bb-10.2-compatibility
>
> Hello Jerome,
>
>
> On 03/14/2017 06:07 PM, jerome brauge wrote:
> > Hi Alexander,
> > Can you review this patch ?
> > I could add more tests when your others points will be corrected.
>
> I tried to simplify your patch slightly.
>
> sp_instr_cpush instructions are now created from the array
> sp_pcontext::m_cursors, which already stores all cursors declared on the
> current frame.
>
> So there is now no a need for a separate list sp_head::m_delayed_cpush.
>
> This seems to work fine.
>
> Also, I changed the meaning of the last argument of sp_declare_cursor() to
> the opposite: from "delay_instr" to "add_cpush_instr".
> I find "bool do_something" easier to read than "bool dont_do_something",
> especially in this context:
>
> if (do_something)
>
> vs
>
> if (!dont_do_something)
>
>
> but this is probably my personal preference :)
>
> Also I removed overloading (and fixed sql_yacc.yy instead).
>
>
> Can you please review the attached patch?
>
> (It also includes my previous suggestions)
>
> Many thanks!!!
>
> >
> > Thanks.
> > Jérôme.
> >
> >
> >> -----Message d'origine-----
> >> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mardi 14 mars
> >> 2017 11:54 À : jerome brauge Cc : MariaDB Developers
> >> (maria-developers@lists.launchpad.net)
> >> Objet : Re: MDEV-10598 - bb-10.2-compatibility
> >>
> >> Hi Jerome,
> >>
> >> On 03/14/2017 02:29 PM, jerome brauge wrote:
> >>> Thanks Alexander,
> >>>
> >>> I have two other problems to finalize my patch:
> >>> - Attached script cur_err_warning.sql produce a warning 1931 (Query
> >> execution was interrupted. The query examined at least 2 rows, which
> >> exceeds LIMIT ROWS EXAMINED (0). The query result may be incomplete)
> >> and I don't see why.
> >>
> >> That's my fault. Please ignore this warning for now.
> >> This warning happens when the procedure opens a cursor to resolve its
> >> structure. I will suppress this warning later.
> >>
> >>
> >>>
> >>> - Attached script cur_err_field_name.sql produce an error 4057
> >>> (HY000) at
> >> line 23: Row variable 'rec2' does not have a field 'a'
> >>
> >> This is also my bug.
> >> The problem is that rec2 has a name "rec1.a" instead of just "a".
> >>
> >>
> >>
> >> Please change
> >>
> >> CURSOR cur2 IS SELECT rec1.a ;
> >>
> >> to
> >>
> >> CURSOR cur2 IS SELECT rec1.a AS a;
> >>
> >>
> >>>
> >>> These two scripts work fine on oracle.
> >>>
> >>> Jérôme.
> >>>
> >>>
> >>>> -----Message d'origine-----
> >>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mardi 14
> >>>> mars
> >>>> 2017 11:11 À : jerome brauge Objet : Re: MDEV-10598 -
> >>>> bb-10.2-compatibility
> >>>>
> >>>> Hello Jerome,
> >>>>
> >>>>
> >>>>
> >>>> On 03/13/2017 07:46 PM, jerome brauge wrote:
> >>>>> Alexander,
> >>>>> I think there is a little bug with ROWTYPE: when I affect a
> >>>>> variable with a
> >>>> field value, the server crash.
> >>>>> I attached the script which cause the issue.
> >>>>
> >>>> Thanks for reporting this.
> >>>>
> >>>>
> >>>> This quick patch fixes the problem:
> >>>>
> >>>>
> >>>> diff --git a/sql/sp_head.cc b/sql/sp_head.cc index faf9f5f..745f982
> >>>> 100644
> >>>> --- a/sql/sp_head.cc
> >>>> +++ b/sql/sp_head.cc
> >>>> @@ -347,6 +347,12 @@ Item *
> >>>> sp_prepare_func_item(THD* thd, Item **it_addr, uint cols) {
> >>>> DBUG_ENTER("sp_prepare_func_item");
> >>>> + if (!(*it_addr)->fixed &&
> >>>> + (*it_addr)->fix_fields(thd, it_addr)) {
> >>>> + DBUG_PRINT("info", ("fix_fields() failed"));
> >>>> + DBUG_RETURN(NULL);
> >>>> + }
> >>>> it_addr= (*it_addr)->this_item_addr(thd, it_addr);
> >>>>
> >>>> if ((!(*it_addr)->fixed &&
> >>>>
> >>>>
> >>>>
> >>>> But I'm still thinking.
> >>>> Perhaps I'll end some with some different patch.
> >>>> I let you know when the final fix is pushed.
> >>>>
> >>>>
> >>>>>
> >>>>> This is the call stack :
> >>>>>
> >>>>>> mysqld.exe!my_sigabrt_handler(int sig) Line 477 C
> >>>>> mysqld.exe!raise(int signum) Line 516 C++
> >>>>> mysqld.exe!abort() Line 64 C++
> >>>>> mysqld.exe!common_assert_to_stderr_direct(const wchar_t
> * const
> >>>> expression, const wchar_t * const file_name, const unsigned int
> >>>> line_number) Line 124 C++
> >>>>> mysqld.exe!common_assert_to_stderr<wchar_t>(const
> wchar_t *
> >>>> const expression, const wchar_t * const file_name, const unsigned int
> >>>> line_number) Line 138 C++
> >>>>> mysqld.exe!common_assert<wchar_t>(const wchar_t *
> const
> >>>> expression, const wchar_t * const file_name, const unsigned int
> >>>> line_number, void * const return_address) Line 383 C++
> >>>>> mysqld.exe!_wassert(const wchar_t * expression, const
> wchar_t *
> >>>> file_name, unsigned int line_number) Line 404 C++
> >>>>>
> mysqld.exe!Item_splocal_row_field_by_name::this_item_addr(THD
> >>>> * thd, Item * * it) Line 1814 C++
> >>>>> mysqld.exe!sp_prepare_func_item(THD * thd, Item * *
> it_addr,
> >>>> unsigned int cols) Line 350 C++
> >>>>> mysqld.exe!sp_eval_expr(THD * thd, Item * result_item,
> Field *
> >>>> result_field, Item * * expr_item_ptr) Line 391 C++
> >>>>> mysqld.exe!sp_rcontext::set_variable(THD * thd, unsigned
> int
> >>>>> idx,
> >>>> Item * * value) Line 566 C++
> >>>>> mysqld.exe!sp_instr_set::exec_core(THD * thd, unsigned int
> *
> >>>> nextp) Line 3378 C++
> >>>>> mysqld.exe!sp_lex_keeper::reset_lex_and_exec_core(THD
> * thd,
> >>>> unsigned int * nextp, bool open_tables, sp_instr * instr) Line 3097
> C++
> >>>>> mysqld.exe!sp_instr_set::execute(THD * thd, unsigned int *
> >>>>> nextp)
> >>>> Line 3371 C++
> >>>>> mysqld.exe!sp_head::execute(THD * thd, bool
> >>>> merge_da_on_success) Line 1261 C++
> >>>>> mysqld.exe!sp_head::execute_procedure(THD * thd,
> List<Item> *
> >>>> args) Line 2086 C++
> >>>>> mysqld.exe!do_execute_sp(THD * thd, sp_head * sp) Line
> 2890
> >>>> C++
> >>>>> mysqld.exe!mysql_execute_command(THD * thd) Line 5919
> C++
> >>>>> mysqld.exe!mysql_parse(THD * thd, char * rawbuf, unsigned
> int
> >>>> length, Parser_state * parser_state, bool is_com_multi, bool
> >>>> is_next_command) Line 8006 C++
> >>>>> mysqld.exe!dispatch_command(enum_server_command
> command,
> >>>> THD * thd, char * packet, unsigned int packet_length, bool
> is_com_multi,
> >>>> bool is_next_command) Line 1821 C++
> >>>>> mysqld.exe!do_command(THD * thd) Line 1369 C++
> >>>>> mysqld.exe!threadpool_process_request(THD * thd) Line
> 346
> >>>> C++
> >>>>> mysqld.exe!tp_callback(TP_connection * c) Line 192 C++
> >>>>> mysqld.exe!tp_callback(_TP_CALLBACK_INSTANCE *
> instance, void *
> >>>> context) Line 377 C++
> >>>>> mysqld.exe!work_callback(_TP_CALLBACK_INSTANCE *
> instance,
> >>>> void * context, _TP_WORK * work) Line 451 C++
> >>>>>
> >>>>> Can you reproduce this ?
> >>>>>
> >>>>> Jérôme.
> >>>>>
> >>>>>> -----Message d'origine-----
> >>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : lundi 13
> >>>>>> mars
> >>>>>> 2017 14:47 À : jerome brauge Objet : Re: MDEV-10598 -
> >>>>>> bb-10.2-compatibility
> >>>>>>
> >>>>>> Jérôme,
> >>>>>>
> >>>>>> On 03/13/2017 05:43 PM, jerome brauge wrote:
> >>>>>>> Hello Alexander,
> >>>>>>> I have to do some changes in the patch and add some tests cases
> >>>>>>> (with row type) I think it will be ready this afternoon (CET).
> >>>>>>
> >>>>>> Excellent. You rock!
> >>>>>>
> >>>>>>>
> >>>>>>> Jérôme.
> >>>>>>>
> >>>>>>>
> >>>>>>>> -----Message d'origine-----
> >>>>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : lundi
> >>>>>>>> 13 mars
> >>>>>>>> 2017 14:38 À : jerome brauge Cc : maria-developers Objet : Re:
> >>>>>>>> MDEV-10598 - bb-10.2-compatibility
> >>>>>>>>
> >>>>>>>> Hello Jerome,
> >>>>>>>>
> >>>>>>>> will you try to apply your patch on top of the current bb-10.2-
> >>>>>> compatibility?
> >>>>>>>>
> >>>>>>>> Or should I do that?
> >>>>>>>>
> >>>>>>>> Thanks!
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 03/10/2017 02:36 PM, Alexander Barkov wrote:
> >>>>>>>>> Hello Jerome,
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On 02/27/2017 11:39 PM, jerome brauge wrote:
> >>>>>>>>>> Hello Alexander,
> >>>>>>>>>> Thanks for the explanation.
> >>>>>>>>>> It's something we do not use. I did not think about it.
> >>>>>>>>>> I look forward to your patch.
> >>>>>>>>>
> >>>>>>>>> I pushed these tasks:
> >>>>>>>>>
> >>>>>>>>> MDEV-10581 sql_mode=ORACLE: Explicit cursor FOR LOOP
> >>>>>>>>> MDEV-12098 sql_mode=ORACLE: Implicit cursor FOR loop
> >>>>>>>>> MDEV-12011 sql_mode=ORACLE: cursor%ROWTYPE in variable
> >>>>>> declarations
> >>>>>>>>> MDEV-12133 sql_mode=ORACLE: table%ROWTYPE in variable
> >>>>>> declarations
> >>>>>>>>>
> >>>>>>>>> Please clone the branch again.
> >>>>>>>>> Git pull will not work, because I recently rebased
> >>>>>>>>> bb-10.2-compatibility on top of the latest 10.2.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> When implementing cursor%ROWTYPE, I had your patch in mind
> >> and
> >>>>>> made
> >>>>>>>>> some refactoring to help us apply MDEV-10598 easier.
> >>>>>>>>> Please see a comment to MDEV-12011 in "git log".
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Now the tricky thing (when adding your patch) is to make sure
> >>>>>>>>> that this work fine:
> >>>>>>>>>
> >>>>>>>>> CREATE PROCEDURE p1
> >>>>>>>>> AS
> >>>>>>>>> a INT:=10;
> >>>>>>>>> CURSOR cur1 IS SELECT a;
> >>>>>>>>> rec1 cur1%ROWTYPE;
> >>>>>>>>> CURSOR cur2 IS SELECT rec1.a;
> >>>>>>>>> rec2 cur2%ROWTYPE;
> >>>>>>>>> BEGIN
> >>>>>>>>> OPEN cur2;
> >>>>>>>>> FETCH cur2 INTO rec2;
> >>>>>>>>> CLOSE cur2;
> >>>>>>>>> SELECT rec2.a;
> >>>>>>>>> END;
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> I.e. a set of intermixed CURSOR and cursor%ROWTYPE variable
> >>>>>>>>> declarations referencing each other recursively.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Thanks.
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Regards,
> >>>>>>>>>> Jérôme.
> >>>>>>>>>>
> >>>>>>>>>>> -----Message d'origine-----
> >>>>>>>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé :
> >>>>>>>>>>> lundi
> >>>>>>>>>>> 27 février 2017 11:28 À : jerome brauge Cc :
> >>>>>>>>>>> maria-developers
> >> Objet :
> >>>>>>>>>>> Re: MDEV-10598 - bb-10.2-compatibility
> >>>>>>>>>>>
> >>>>>>>>>>> Hello Jerome,
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> On 02/21/2017 07:18 PM, jerome brauge wrote:
> >>>>>>>>>>>> Hello Alexander,
> >>>>>>>>>>>> I've done this patch for MDEV-10598.
> >>>>>>>>>>>> Can you review it ?
> >>>>>>>>>>>
> >>>>>>>>>>> It seems we'll have to postpone this patch.
> >>>>>>>>>>>
> >>>>>>>>>>> I'm currently working on:
> >>>>>>>>>>>
> >>>>>>>>>>> MDEV-10598 Variable declarations can go after cursor
> >>>>>>>>>>> declarations
> >>>>>>>>>>> MDEV-12011 sql_mode=ORACLE: cursor%ROWTYPE in variable
> >>>>>>>> declarations
> >>>>>>>>>>> So the trick with postponing variable declarations using a
> >>>>>>>>>>> temporary list might not work properly after adding
> >>>>>>>>>>> MDEV-10598 abd MDEV-12011, the order of cursors and
> variables is important.
> >>>>>>>>>>>
> >>>>>>>>>>> Example:
> >>>>>>>>>>>
> >>>>>>>>>>> DECLARE
> >>>>>>>>>>> CURSOR cur1 IS SELECT a,b FROM t1;
> >>>>>>>>>>> v cur1%ROWTYPE;
> >>>>>>>>>>> CURSOR cur2 IS SELECT v.a, v.b FROM DUAL; BEGIN
> >>>>>>>>>>> ...
> >>>>>>>>>>> END;
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> So the order of cur1, v and cur2 is important.
> >>>>>>>>>>>
> >>>>>>>>>>> I'll let you known when I'm ready with %ROWTYPE tasks.
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks!
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Regards,
> >>>>>>>>>>>> Jérôme.
> >>>>>>>>>>>>