From: Lars-Erik Bjørk Date: February 10 2009 8:14pm Subject: Re: bug#34478 List-Archive: http://lists.mysql.com/falcon/503 Message-Id: MIME-Version: 1.0 Content-Type: text/plain; delsp=yes; format=flowed; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Wow, that commit message came out a little strange :) On Feb 10, 2009, at 9:07 PM, Vladislav Vaintroub wrote: > if (len < node1->keyLength) > =09=09 { > =09=09=09=09int n =3D checkTail(len, node1); > =09=09=09=09=09 > =09=09=09=09=09=09if (n) > =09=09=09=09=09=09=09=09return n; > =09=09=09=09=09=09=09=09=09 } > > else if (len < node2->keyLength) > > { > > int n =3D 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 ar= e >>>> equal >>>> up to the length of the shortest, and it is partial. Otherwise i= t >>>> 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 li= ne >>>> leads >>>> me to believe that wen only use 0x00 for the mentioned padding >>>> anyway: >>>> >>>> void Field::setCollation(Collation *newCollation) >>>> { >>>> collation =3D newCollation; >>>> //indexPadByte =3D collation->getPadChar(); >>>> indexPadByte =3D 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 v= ery >>>> 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 th= e >>>> bugs >>>> are definitively closely related. >>>> >>>> Best, >>>> >>>> Lars-Erik >>>> >>>> Vladislav Vaintroub wrote: >>>>> I guess varbinary does not have any padding and this is the = =20 >>>>> clue... >>>>> >>>>> The compare function for index keys does not exist as of today. >>>>> >>>>> It could look like >>>>> >>>>> diff =3D memcmp(a, b, min(length_a,length_b)); >>>>> if (diff =3D=3D 0) >>>>> diff =3D (length_a > length_b)?1: ((length_a < length_b):-1:0); >>>>> >>>>> >>>>> What makes me wonder in this bug is that IndexPage is not healt= hy >>>> either. >>>>> >>>>> This could be effect of optimization that has been in index cod= e >>>> since ever: >>>>> when merging a new key, use the fact that keys in log record ar= e >>>> 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=F8rk >>>>>> Cc: FalconDev >>>>>> Subject: Re: bug#34478 >>>>>> >>>>>> What about making the DeferredIndex class sort things exactly = =20 >>>>>> like >>>> the >>>>>> IndexKeys read from index pages? Can it use the same compare >>>> function? >>>>>> >>>>>> Lars-Erik Bj=F8rk wrote: >>>>>> >>>>>>> Hi all! >>>>>>> >>>>>>> I don't have the the battery capacity to write a long mail no= w >>>> (lucky >>>>>>> you!) :) I analyzed bug#34478 ( >>>>>>> >>>>>> http://bugs.mysql.com/bug.php?id=3D34478) >>>>>> >>>>>>> which also narrows down to the use of 0x00 in index keys (thi= s >>>>>>> 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 i= t >>>>>>> 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=3Dwlad@stripped >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Falcon Storage Engine Mailing List >>>>> For list archives: http://lists.mysql.com/falcon >>>>> To unsubscribe: http://lists.mysql.com/falcon?unsub=3Dlars- >>>> erik.bjork@stripped >>>>> >>>>> >>>> >>>> >>>> -- >>>> Falcon Storage Engine Mailing List >>>> For list archives: http://lists.mysql.com/falcon >>>> To unsubscribe: http://lists.mysql.com/falcon?unsub=3Dwlad@su= n.com >>> >>> >>> >>> -- >>> Falcon Storage Engine Mailing List >>> For list archives: http://lists.mysql.com/falcon >>> To unsubscribe: http://lists.mysql.com/falcon?unsub=3Dlars- >> erik.bjork@stripped >>> >> >> >> -- >> Falcon Storage Engine Mailing List >> For list archives: http://lists.mysql.com/falcon >> To unsubscribe: http://lists.mysql.com/falcon?unsub=3Dwlad@sun.= com > >