List:Falcon Storage Engine« Previous MessageNext Message »
From:Lars-Erik Bjørk Date:February 10 2009 8:14pm
Subject:Re: bug#34478
View as plain text  
Wow, that commit message came out a little strange :)


On Feb 10, 2009, at 9:07 PM, Vladislav Vaintroub wrote:

> if (len < node1->keyLength)
>      		   {
>      				int n = checkTail(len, node1);
>      					
>      						if (n)
>      								return n;
>      									   }
>
> else if (len < node2->keyLength)
>
> {
>
> int n = checkTail(len, node2);
>
>
>
> if (n)
>
> return -n;
>
> }
>
> People think "use decent editor";) Was not Visual Studio available on
> Macs???
>
>
>> -----Original Message-----
>> From: Lars-Erik.Bjork@stripped [mailto:Lars-Erik.Bjork@stripped]
>> Sent: Tuesday, February 10, 2009 9:05 PM
>> To: Vladislav Vaintroub
>> Cc: Kevin Lewis; FalconDev; Jim Starkey
>> Subject: Re: bug#34478
>>
>> I have committed a patch based on this, what does people think?
>>
>> http://lists.mysql.com/commits/65802
>>
>> /Lars-Erik
>>
>>
>> On Feb 10, 2009, at 1:20 PM, Vladislav Vaintroub wrote:
>>
>>> checkTail()/padding do not make sense to me. I would skip it or
>>> better yet,
>>> delete and see what happens.
>>>
>>>
>>>> -----Original Message-----
>>>> From: Lars-Erik.Bjork@stripped [mailto:Lars-Erik.Bjork@stripped]
>>>> Sent: Tuesday, February 10, 2009 1:16 PM
>>>> To: Vladislav Vaintroub
>>>> Cc: Kevin.Lewis@stripped; 'FalconDev'
>>>> Subject: Re: bug#34478
>>>>
>>>> Looking at the compare methods for the IndexKey class (used in
>>>> IndexPage::indexMerge), none of them pad the shorter key to the
>>>> length
>>>> of the longer. They only compare on record number if the keys are
>>>> equal
>>>> up to the length of the shortest, and it is partial. Otherwise it
>>>> compares on key length.
>>>>
>>>> Would this make sense for the DeferredIndex::compare methods as
>> well?
>>>> Skip the checkTails() calls? Or will this make types with a
>> different
>>>> pad character than 0x00 sort incorrectly.
>>>>
>>>> The following method with the (for some reason) commented out line
>>>> leads
>>>> me to believe that wen only use 0x00 for the mentioned padding
>>>> anyway:
>>>>
>>>> void Field::setCollation(Collation *newCollation)
>>>> {
>>>>   collation = newCollation;
>>>>   //indexPadByte = collation->getPadChar();
>>>>   indexPadByte = 0;
>>>> }
>>>>
>>>>
>>>> I am not so familiar with what is the correct padding character and
>>>> when
>>>> to pad etc. in all cases for every type, although the topic is very
>>>> fascinating ;)
>>>>
>>>> Btw, Kevin, I tried removing the padding, and that seems to fix the
>>>> bug
>>>> you linked to (bug#33190), when enabling the assert again. So the
>>>> bugs
>>>> are definitively closely related.
>>>>
>>>> Best,
>>>>
>>>> Lars-Erik
>>>>
>>>> Vladislav Vaintroub wrote:
>>>>> I guess varbinary does not have any padding and this is the  
>>>>> clue...
>>>>>
>>>>> The compare function for index keys does not exist as of today.
>>>>>
>>>>> It could look like
>>>>>
>>>>> diff = memcmp(a, b, min(length_a,length_b));
>>>>> if (diff == 0)
>>>>> diff = (length_a > length_b)?1: ((length_a < length_b):-1:0);
>>>>>
>>>>>
>>>>> What makes me wonder in this bug is that IndexPage is not healthy
>>>> either.
>>>>>
>>>>> This could be effect of optimization that has been in index code
>>>> since ever:
>>>>> when merging a new key, use the fact that keys in log record are
>>>> presorted -
>>>>>
>>>>> findInsertionPoint does not search from the start of the page, but
>>>> from the
>>>>> position where prior key has been inserted.
>>>>> And if keys are wrongly presorted, IndexPage could be easily
>>>> corrupted too.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Kevin.Lewis@stripped [mailto:Kevin.Lewis@stripped]
>>>>>> Sent: Monday, February 09, 2009 9:46 PM
>>>>>> To: Lars-Erik Bjørk
>>>>>> Cc: FalconDev
>>>>>> Subject: Re: bug#34478
>>>>>>
>>>>>> What about making the DeferredIndex class sort things exactly  
>>>>>> like
>>>> the
>>>>>> IndexKeys read from index pages?  Can it use the same compare
>>>> function?
>>>>>>
>>>>>> Lars-Erik Bjørk wrote:
>>>>>>
>>>>>>> Hi all!
>>>>>>>
>>>>>>> I don't have the the battery capacity to write a long mail
> now
>>>> (lucky
>>>>>>> you!) :) I analyzed bug#34478 (
>>>>>>>
>>>>>> http://bugs.mysql.com/bug.php?id=34478)
>>>>>>
>>>>>>> which also narrows down to the use of 0x00 in index keys
> (this
>>>>>>> time
>>>>>>>
>>>>>> as
>>>>>>
>>>>>>> pad bytes in the deferred index). Does anyone have an opinion
> on
>>>> how
>>>>>>> this bug can be avoided in a nice and correct fashion? Does
> it
>>>>>>> make
>>>>>>>
>>>>>> any
>>>>>>
>>>>>>> sense to pad with 0x00?
>>>>>>>
>>>>>>> 2 minutes capacity left, sorry the short mail, please see
> the
>>>> report
>>>>>>>
>>>>>> for
>>>>>>
>>>>>>> a bit more elaborate version :)
>>>>>>>
>>>>>>> Best,
>>>>>>>
>>>>>>> Lars-Erik
>>>>>>>
>>>>>>>
>>>>>> --
>>>>>> Falcon Storage Engine Mailing List
>>>>>> For list archives: http://lists.mysql.com/falcon
>>>>>> To unsubscribe:
>> http://lists.mysql.com/falcon?unsub=1
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Falcon Storage Engine Mailing List
>>>>> For list archives: http://lists.mysql.com/falcon
>>>>> To unsubscribe:    http://lists.mysql.com/falcon?unsub=1
>>>> erik.bjork@stripped
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Falcon Storage Engine Mailing List
>>>> For list archives: http://lists.mysql.com/falcon
>>>> To unsubscribe:    http://lists.mysql.com/falcon?unsub=1
>>>
>>>
>>>
>>> --
>>> Falcon Storage Engine Mailing List
>>> For list archives: http://lists.mysql.com/falcon
>>> To unsubscribe:    http://lists.mysql.com/falcon?unsub=1
>> erik.bjork@stripped
>>>
>>
>>
>> --
>> Falcon Storage Engine Mailing List
>> For list archives: http://lists.mysql.com/falcon
>> To unsubscribe:    http://lists.mysql.com/falcon?unsub=1
>
>

Thread
bug#34478Lars-Erik Bjørk9 Feb
  • Re: bug#34478Kevin Lewis9 Feb
    • RE: bug#34478Vladislav Vaintroub9 Feb
      • Re: bug#34478Lars-Erik Bjørk10 Feb
        • RE: bug#34478Vladislav Vaintroub10 Feb
          • Re: bug#34478Lars-Erik Bjørk10 Feb
            • RE: bug#34478Vladislav Vaintroub10 Feb
              • Re: bug#34478Lars-Erik Bjørk10 Feb
                • RE: bug#34478Vladislav Vaintroub10 Feb
                  • Re: bug#34478Lars-Erik Bjørk11 Feb