Hi! Thank you for the fast response! I feel obliged to return the favor!
Roy Lyseng skrev 2011-02-15 15.07:
>>>> +SELECT a, AVG(t1.b),
>>>> +(SELECT t11.c FROM t1 t11 WHERE t11.a = t1.a AND t11.b =
>>>> AVG(t1.b)) AS t11c,
>>>> +(SELECT t12.c FROM t1 t12 WHERE t12.a = t1.a AND t12.b =
>>>> AVG(t1.b)) AS t12c
>>>> +FROM t1 GROUP BY a;
>>> I'd suggest that you write something like:
>>> set $query=SELECT ...;
>>> eval EXPLAIN $query;
>>> eval $query;
>> Well personally I don't like that style and haven't seen anybody else
>> using it.
> ...but you did not challenge my argument :)
Nope, and that's on purpose. :-) My only argument is taste. It's more
writing and you are forced to break lines after less SQL, which I don't
> Anyway, this was just a suggestion.
>> If anything, we should adopt the new explain mode in mysql_test_run.
> I'll wait to see how that mode turns out. Is it maintainable?
You might ask Tor about it, haven't heard anything in a long time.
>>>> + Item_direct_ref)
>>>> + - fixes references (Item_ref objects) to these fields.
>>> fields -> items
>> Not sure about that one. Can you prove it isn't always fields?
> Nope, this was more of a wild guess :D
And besides you already approved the patch so I can do as I please.
>>> @return true if an error occurred, false if success.
>> Us humans can understand that just fine, but poor Doxygen can't.
> This return convention is so common, that I would advice you to write:
> Personally, I do not care that much about what Doxygen might think...
Yes, it seems I'm the only one poking around in the Doxygen pages :')
>>>> + @todo This comment must be restructured. It is jumping back and
>>>> + between abstraction levels and it uses inconsistent terminology.
>>> Hey! There is actual documentation here! You can either remove the
>>> @todo, fix
>>> the comment, or pinpoint the jumping and use of inconsistent
>> My apologies, didn't mean to offend anyone. I realize that this
>> function has
>> been gradually edited from its initial appearance. In order to make
>> amends I'll
>> try to do all three!
>> It looks like the original author was using the term "Sum" instead of
>> And someone (perhaps you) had been kind enough to change it to
>> "aggregate" in
>> most places, so I fixed the rest of them.
>> So I try leaving it in a little better state than I found it. But I
>> think making
>> the comment 100% correct is beyond a bug fix.
>> In my defense I'll add that this line was not meant to go in the
>> patch. It was
>> more like a memento for me that kinda slipped by.
>>> I guess the inconsistet terminology issues are: field vs. item.
>>> select vs
>>> query block (I prefer query specification or query expression, they
>>> are SQL
>>> official terms),
>> Yes, and sum versus aggregate, fix versus resolve, and what does
>> "pull up" mean?
>> And the comment is jumping back and forth between SQL terminology,
>> design and class layout and list structures, etc.
> Nevertheless, the description is almost spot-on.
I take your word for it :-)
>> But it isn't as simple as search-and-replace either. For instance I
>> suspect that
>> "fix" means different things in different sentences.
> It goes without saying. You cannot fix an inconsistent description by
> applying regular principles.
Well if it's as simple as replacing "sum" with "aggregate" and "fix"
with "resolve" you can...which is basically what I did...
>>>> + */
>>>> fix_inner_refs(THD *thd, List<Item> &all_fields, SELECT_LEX
>>>> @@ -328,11 +336,11 @@ fix_inner_refs(THD *thd, List<Item> &all
>>>> Item_outer_ref *ref;
>>>> bool res= FALSE;
>>> The local variable res is unused and can be removed.
>> Not the bug strictly speaking, but since it gives me an excuse to
>> change FALSE
>> to false I'll add it in. :-)
> This is the kind of minor, local refactoring I think that we can apply
> as part of regular bug fixing. It has (almost) zero risk, and is not
> easy to confuse with the actual bug fix.
I agree. Igor would disagree, but he's not here anymore is he? ;-)