List:Falcon Storage Engine« Previous MessageNext Message »
From:Vladislav Vaintroub Date:February 10 2009 9:05pm
Subject:RE: bug#34478
View as plain text  
Thanks Lars-Eik,
Other then strange alignment in comment, I'm comfortable with this change.

> -----Original Message-----
> From: Lars-Erik.Bjork@stripped [mailto:Lars-Erik.Bjork@stripped]
> Sent: Tuesday, February 10, 2009 9:15 PM
> To: Vladislav Vaintroub
> Cc: 'Kevin Lewis'; 'FalconDev'; 'Jim Starkey'
> Subject: Re: bug#34478
> 
> 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