maria-developers team mailing list archive

[jerome brauge <j.brauge@xxxxxxxxxxx>]

Hi Alexander,
Can you review this patch ?
I could add more tests when your others points will be corrected.

Thanks.
Jérôme.


> -----Message d'origine-----
> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx]
> Envoyé : mardi 14 mars 2017 11:54
> À : jerome brauge
> Cc : MariaDB Developers (maria-developers@xxxxxxxxxxxxxxxxxxx)
> 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@xxxxxxxxxxx] 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@xxxxxxxxxxx] 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@xxxxxxxxxxx] 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@xxxxxxxxxxx] 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.
> >>>>>>>>>>

Attachment: 0001-MDEV-10598-Variable-declarations-can-go-after-cursor.patch
Description: 0001-MDEV-10598-Variable-declarations-can-go-after-cursor.patch