Georgi Kodinov skrev:
> Hi,
>
> On 09.11.2009, at 13:02, Martin Hansson wrote:
>
>>
>> Ok, I may have understood this completely backwards, but could you
>> say that the following is true: Previously the equality propagation
>> procedure assumed that an expression over ROW functions, e.g.
>> ROW(...) <op> ROW(...) wronfully assumed the operator to be '='?
>
> No : the problem was that the SELECT_LEX::max_equal_elems was set to
> garbage because of wrong class cast when dealing with equality
> propagation in ROW(..) = ROW(...) type of predicates.
Yep, I found out I was wrong, but more importantly why I was wrong. It
is because the condition NOT a >= b OR NOT ROW(b,a )<> ROW(a,a) gets
de-Morganed internally to a < b AND ROW(b,a ) = ROW(a,a) (hence it is
equality once we get to the patched code). I did not not-elimination
happened before this step and it is obviously important for the bug. So
it's not due to any assumptions about the logical operators but a plain
coding mistake. (and lack of warnings for illegal casts)
>> Is there a non-obvious reason for the asserts? In my (limited)
>> understanding they only serve to ensure that item_equal is of the
>> class Item_equal (through our poor-man's RTTI.) But
>> COND_EQUAL::current_level is an instantiated template of
>> List<Item_equal>, hence only Item_equal's can go in it anyway. Am I
>> missing something?
>
> No hidden meaning : I've added an assert to check the fact that we're
> assuming in this piece of code : that cond_equal holds only Item_equal
> instances.
> Yes, it can be considered a little superfluous, but better be safe
> than sorry.
> I'll leave it to you to decide whether it's actually needed.
I tend to trust templates. Their only job is to ensure exactly what you
are ensuring with assertions. If we were to second-guess our type
paremeters every time we rely on them we would get a bloated code
indeed. So whatever authority I have on the matter I will put towards
removing the assertions.
>
>> ...
>
> OK, I can do this. I don't know why everybody dislike the else clause.
> To me it's more explicit than if () { ... return; } ... return;.
> And it's a little bit more easier to understand as well.
Yeah, I have no problems with else return either, but I do find code
with a consistent style easier to live with. And it _is_ in our coding
standard.
Regards,
Martin
