List:Commits« Previous MessageNext Message »
From:Evgeny Potemkin Date:December 2 2009 3:50pm
Subject:Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508
View as plain text  
Hi Oystein,

Øystein Grøvlen wrote:
> Evgeny Potemkin wrote:
>> Hi Oystein,
>>
>> Øystein Grøvlen wrote:
>>> Jørgen Løland wrote:
>>>>> === modified file 'sql/sql_class.h'
>>>>> --- a/sql/sql_class.h    2009-10-20 04:42:10 +0000
>>>>> +++ b/sql/sql_class.h    2009-12-01 18:28:45 +0000
>>>>> @@ -759,6 +759,8 @@ public:
>>>>>    { return state == INITIALIZED_FOR_SP; }
>>>>>    inline bool is_stmt_prepare_or_first_sp_execute() const
>>>>>    { return (int)state < (int)PREPARED; }
>>>>> +  inline bool is_stmt_prepare_or_first_stmt_execute() const
>>>>> +  { return (int)state <= (int)PREPARED; }
>>>>
>>>>
>>>> This is nitpicking, but I still don't think the function name 
>>>> reflects the behavior. The name gives the impression that it returns 
>>>> true for PS prepare or PS first execute only, but it also returns 
>>>> true for SP first execute. However, I don't have a good suggestion 
>>>> for another name.
>>>
>>> How about is_stmt_prepare_or_first_execute()?
>> Which is basically the same, but having ...first_stmt_execute there is 
>> the clear
>> difference between this function and another.
> 
> But ...first_stmt_execute is misleading since it does not tell that it 
> also returns true for first execution of stored procedure.
Yep, but I think is_stmt_prepare_or_first_stmt_or_sp_execute is overcomplicated.
> 
>>>
>>> It is not quite clear to me what behavior is wanted with respect to 
>>> stored procedure.  Where the new function is used, the comment says:
>>> "in PS use own arena or data will be freed after prepare".  What 
>>> about SP?
>> Same with SP as INITIALIZED_FOR_SP is less than PREPARED.
> 
> Exactly my point.  Either SP should be added to the comment, or the new 
> function should not return true for INITIALIZED_FOR_SP.  My question is 
> which one is correct.
It should return TRUE. The purpose of this function is to tell whether should we
save items/do optimizations. This is done on the prepare & first execute for PS 
and initialization for SP. For both cases this function works well.
> 
>>>
>>> I have a feeling that it would not hurt use in Item_subselect, too.
>> Probably, but I prefer to not touch it without a test case.
> 
> OK, but I think the ideal situation would be to have only one function 
> is_stmt_prepare_or_first_execute (the new one), and drop the old one 
> entirely.
I agree, but I don't like patching the code without having a test case.
Most probably it would create problems.
> 

Regards, Evgen.
Thread
bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Evgeny Potemkin1 Dec
  • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Jørgen Løland2 Dec
    • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Øystein Grøvlen2 Dec
      • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Evgeny Potemkin2 Dec
        • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Øystein Grøvlen2 Dec
          • Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508Evgeny Potemkin2 Dec