List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:December 2 2009 2:27pm
Subject:Re: bzr commit into mysql-5.0-bugteam branch (epotemkin:2850) Bug#48508
View as plain text  
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.

>>
>> 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.

>>
>> 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.

-- 
Øystein
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