MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:March 11 2010 4:35pm
Subject:Re: bzr commit into mysql-6.0-codebase branch (guilhem:3874)
View as plain text  
Hello Oystein,

Evgeny Potemkin a écrit, Le 11.03.2010 17:27:
> Hi Guilhem, Øystein,
> 
> On 03/11/10 17:58, Guilhem Bichot wrote:
>> Hello Evgeny,
>>
>> Evgeny Potemkin a écrit, Le 11.03.2010 11:24:
>>> Hi Guilhem, Øystein,
>>>
>>> On 03/09/10 18:08, Guilhem Bichot wrote:
>>>> Hello,
>>>> (Evgeny, there is a question for you below, please search for 
>>>> "Evgeny"),
>>>>
>>>> Øystein Grøvlen a écrit, Le 02.03.2010 13:00:
>>>>> Hi Guilhem,
>>>>>
>>>>> Thanks for documenting what you found. I have some
> comments/questions
>>>>> inline.
>>>>>
>>>>> Guilhem Bichot wrote:
>>>>>> #At
>>>>>>
> file:///home/mysql_src/bzrrepos/mysql-6.0-codebase-bugfixing-50361/
>>>>>> based on revid:guilhem@stripped
>>>
>>> [skip]
>>>
>>>>>> === modified file 'sql/sql_lex.h'
>>>>>> --- a/sql/sql_lex.h 2010-02-05 16:57:46 +0000
>>>>>> +++ b/sql/sql_lex.h 2010-02-24 09:11:25 +0000
>>>>>> @@ -557,9 +557,15 @@ public:
>>>>>> SQL_LIST group_list; /* GROUP BY clause. */
>>>>>> List<Item> item_list; /* list of fields & expressions
> */
>>>>>> List<String> interval_list;
>>>>>> + /**
>>>>>> + Name resolution in Item_field::fix_fields() searches for a name
> in
>>>>>> + tables' columns' names; if this boolean is true, it is allowed
> to
>>>>>> + additionally look up in SELECT's item_list (necessary to find
>>>>>> aliased
>>>>>> + fields).
>>>>>> + */
>>>>>
>>>>> Why is "is allowed to" more appropriate than "needs to"?

Based on Evgeny's reply below, I suggest to write it as
/**
Name resolution in Item_field::fix_fields() searches for a name in
tables' columns' names; if this boolean is true, it is allowed to
additionally look up in SELECT's item_list (necessary to find
aliased fields). Examples: boolean is true for a GROUP BY clause (which 
can refer to aliases), false for a WHERE clause (which mustn't).
*/

Is that ok?
After that, I'll commit a new patch.

>>>>
>>>> It "needs to" look into item_list only if there are fields which it
>>>> cannot find in tables.
>>>> It is allowed to look into item_list.
>>>> In other places, I guess it would be wrong to look into item_list, when
>>>> only real column names are wanted?
>>>> Which is why "allowed to" would be correct here?
>>>> This member was introduced for
>>>>
>>>> http://bugs.mysql.com/bug.php?id=7672
>>>>
>>>> (Evgeny's fix), with this comment:
>>>>
>>>> "When fixing Item_func_plus in ORDER BY clause field c is searched in
>>>> all opened tables, but because c is an alias it wasn't found there."
>>>> It's tested in Item_field::fix_fields().
>>>> Evgeny, please, could you give a good comment for this member?
>>>>
>>>>> "tables' columns' names" was a bit difficult to parse.
>>>>
>>>> ok
>>>>
>>>>>> bool is_item_list_lookup;
>>> This thing is needed for resolving ORDER BY (and, AFAIR, GROUP BY) 
>>> items.
>>> For example
>>> SELECT a, b AS f2 FROM t1 ORDER BY a, func(f2)
>>> 'a' will be successfully resolved against t1, but f2 won't. For such
>>> cases
>>> it's allowed for ORDER BY to do item list look up to find items.
>>
>> and it's even "needed".
>>
>>> In case of 'a' table look up is enough, so ORDER BY doesn't "need to"
>>> do look at item list.
>>
>> Thanks.
>> On several places in code, we set is_item_list_lookup to false; I wonder
>> if it is a case where looking up in the item_list is:
>> - not "needed" (resolution will surely succeed without looking up in the
>> list, and would succeed with looking up too)
>> - not "allowed" (resolution would do something wrong if it looked into
>> the list)?
> Not allowed. For example - idents from WHERE clause should refer only to 
> real tables/views. This means that name resolution code isn't allowed to 
> look at aliases in the item list.
>> Depending on your reply, I'll be able to clarify the comment which I
>> plan to add.
>>
> Regards, Evgen.

Thread
bzr commit into mysql-6.0-codebase branch (guilhem:3874) Guilhem Bichot24 Feb
  • Re: bzr commit into mysql-6.0-codebase branch (guilhem:3874)Øystein Grøvlen2 Mar
    • Re: bzr commit into mysql-6.0-codebase branch (guilhem:3874)Guilhem Bichot9 Mar
      • Re: bzr commit into mysql-6.0-codebase branch (guilhem:3874)Evgeny Potemkin11 Mar
        • Re: bzr commit into mysql-6.0-codebase branch (guilhem:3874)Guilhem Bichot11 Mar
          • Re: bzr commit into mysql-6.0-codebase branch (guilhem:3874)Evgeny Potemkin11 Mar
            • Re: bzr commit into mysql-6.0-codebase branch (guilhem:3874)Guilhem Bichot11 Mar
        • Re: bzr commit into mysql-6.0-codebase branch (guilhem:3874)Roy Lyseng11 Mar
  • Re: bzr commit into mysql-6.0-codebase branch (guilhem:3874)Øystein Grøvlen17 Mar