Re: bzr commit into mysql-5.1-bugteam branch (v.narayanan:3197) Bug#47762

[Kevin Lewis]

V Narayanan,

Your new testcase result file contains;
=======================================
+CREATE TABLE t1 (a int(11), key k2 (a)) engine=innodb;
+Warnings:
+Warning	1286	Unknown table engine 'innodb'
+Warning	1266	Using storage engine MyISAM for table 't1'
=======================================

You don't want to test for a missing innodb do you?

Also, the test name is key.test and it seems to be in the main test suite.
I think that is too generic of a test name.  It should probably have 'min' and 'null' in
the test name.

Also, if you are developing a new testcase, why not issue the same SQL on a table with a
not null index, and again with a table having no index... Just to make sure everything
continues to work as expected in the future.

Kevin

V Narayanan wrote:
> Hi,
> 
> In the previous patch I submitted I found failures in 
> ndb_dd_sql_features.test.
> There was a server crash in this test because cond could be NULL in 
> reckey_in_range
> and I had not checked for it. I have submitted a new patch that does this
> test. I have also run the entire suite on this patch. All the tests 
> pass. I have
> also introduced new tests using the cases given by Kevin, this I had done
> in my previous submission also.
> 
> Thank You,
> Narayanan
> 
> V Narayanan wrote:
>> Hi,
>>
>> I have committed a new patch along the suggestions given by Serg. In this
>> patch I have handled NULLs separately.
>>
>> Thank you Jorgen, Kevin and Serg for your guidance.
>>
>> Narayanan
>>
>>
>> Sergei Golubchik wrote:
>>> Hi, VN !
>>>
>>> On Oct 30, V Narayanan wrote:
>>>  
>>>>  3197 V Narayanan    2009-10-30
>>>>       Bug#47762 Incorrect result from MIN() when WHERE tests NOT NULL
>>>>       column for NULL
>>>>             The lookup of a NULL value in a column having a NOT NULL
>>>>       index was resulting in wrong query result. This was happening
>>>>       due to the return value, from the function that stored the
>>>>       field value, being ignored.
>>>>     
>>>
>>> Okay.
>>>
>>> The main problem here is that there is no special handling of
>>> comparisons with NULL in opt_sum_query().
>>>
>>> When you compare a not-nullable field with a NULL,
>>> set_field_to_null_with_conversions() fails - although, in fact it should
>>> not fail, it's called with CHECK_FIELD_IGNORE, which is a bug on
>>> itself - and the record is not updated, it holds the old (basically,
>>> arbitrary) value which is used later in key lookups. MySQL trust the
>>> index lookup here and does not verify that the found value, indeed,
>>> satisfies WHERE condition. Your changeset fixes this special case.
>>>
>>> When you compare a nullable field with a NULL, NULL is stored in the
>>> record and it's looked up in the index. If there's no matching record
>>> (with NULL) in the table, as in Kevin's test case, MySQL rightfully
>>> decides that no record was found.
>>>
>>> If there is a matching record (add INSERT t2 VALUES (NULL) to Kevin's
>>> test), it *is* found and returned. But as it contains NULL, min(NULL) is
>>> NULL, and the result appears to be correct, even if it was found using
>>> the completely wrong logic :)
>>>
>>> Well, if you use > NULL - as Kevin has shown - that wrong logic breaks
>>> and no longer delivers correct results.
>>>
>>> Therefore, I think the correct and complete solution to this bug and
>>> other anomalies is to treat NULLs specially in opt_sum_query().
>>>
>>> Regards / Mit vielen Grüßen,
>>> Sergei
>>>
>>>   
>>
>>
> 
>