List:Falcon Storage Engine« Previous MessageNext Message »
From:Vladislav Vaintroub Date:February 16 2009 9:36pm
Subject:RE: Patch for bug#42208
View as plain text  
Hi Lars-Erik,

I think I missed one point.

We need to increase ODS version (ODS_MINOR_VERSION I guess), as the change
to IndexKeys makes existing tablespaces incompatible.

And again on mixing 0x00 and empty strings. I think removing the whole scary
workaround of caching of empties and NULLs in IndexWalker  and replace with
following (or similar) at the end of Index::makeKey would be better  (and
generally less workarounds is good). Let's see what other reviewers say.

Index.cpp , void Index::makeKey(Field *field, Value *value, int segment,
IndexKey *indexKey) , 
...

	// Move empty keys -> 0x00 and prepend extra 0x00 to key already
starting with 0x00
	// This transformation is done to distinguish NULLs from anything
else and it
	// preserves the original sorting sequence.
	if (indexKey->keyLength == 0)
		{
		indexKey->key[0] = 0;
		indexKey->keyLength = 1;
		}
	else if(indexKey->key[0] == 0)
		{
		size_t moveLen = MIN(indexKey->keyLength,
PHYSICAL_KEY_LENGTH - 1);
		memmove(indexKey->key + 1,indexKey->key, moveLen);
		indexKey->key[0] = 0;
		indexKey->keyLength = moveLen + 1;
		}

But nonetheless, it looks like a great investigation of IndexWalker guts , I
guess I know who'll fix the nasty PB2 limit bugs :)



> -----Original Message-----
> From: Vladislav.Vaintroub@stripped [mailto:Vladislav.Vaintroub@stripped]
> On Behalf Of Vladislav Vaintroub
> Sent: Monday, February 16, 2009 9:50 PM
> To: Lars-Erik.Bjork@stripped; 'FalconDev'
> Subject: RE: Patch for bug#42208
> 
> Hi Lars-Erik,
> I wonder if adding 0x00 to the (binary) string values that already
> start
> with 0x00 would not be less works that modifying index walker etc. This
> looks like huge amount of work you have done (good) but I wonder if
> there is
> a good reason for it. Assuming (binary) strings that start with 0x00
> are
> really seldom, prepending 0x00 to a key after a check is not going to
> be an
> expensive operation. And that makes NULL *really* different from other
> index
> values. And that allows maybe in some distant future index-only access,
> so
> you can answer "is null/is not null" without extra accessing the record
> and
> this is a real performance advantage.
> 
> 
> > -----Original Message-----
> > From: Lars-Erik.Bjork@stripped [mailto:Lars-Erik.Bjork@stripped]
> > Sent: Monday, February 16, 2009 8:46 PM
> > To: FalconDev
> > Subject: Patch for bug#42208
> >
> > I have committed a patch for bug#42208 which is available - and
> > explained at -
> > http://lists.mysql.com/commits/66541
> >
> >
> > Question: Do we reuse index walkers? Or do we traverse only once?
> > Should I reset cache counters etc?
> >
> > /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


Thread
Patch for bug#42208Lars-Erik Bjørk16 Feb
  • RE: Patch for bug#42208Vladislav Vaintroub16 Feb
    • RE: Patch for bug#42208Vladislav Vaintroub16 Feb
      • Re: Patch for bug#42208Jim Starkey16 Feb
        • RE: Patch for bug#42208Vladislav Vaintroub16 Feb
    • Re: Patch for bug#42208Jim Starkey16 Feb
      • RE: Patch for bug#42208Vladislav Vaintroub16 Feb
      • Re: Patch for bug#42208Lars-Erik Bjørk17 Feb
        • Re: Patch for bug#42208Kevin Lewis17 Feb
          • Re: Patch for bug#42208Ann W. Harrison18 Feb
            • Re: Patch for bug#42208Ann W. Harrison18 Feb
              • Re: Patch for bug#42208Ann W. Harrison18 Feb
                • Re: Patch for bug#42208Kevin Lewis18 Feb
                  • Re: Patch for bug#42208Ann W. Harrison18 Feb
                    • Re: Patch for bug#42208Kevin Lewis18 Feb
          • RE: Patch for bug#42208Vladislav Vaintroub18 Feb
            • RE: Patch for bug#42208Vladislav Vaintroub18 Feb
              • Re: Patch for bug#42208Kevin Lewis18 Feb
                • RE: Patch for bug#42208Vladislav Vaintroub18 Feb
                  • RE: Patch for bug#42208Vladislav Vaintroub18 Feb
              • Re: Patch for bug#42208Jim Starkey18 Feb
                • RE: Patch for bug#42208Vladislav Vaintroub18 Feb
                  • Re: Patch for bug#42208Jim Starkey18 Feb
                    • RE: Patch for bug#42208Vladislav Vaintroub18 Feb
                      • RE: Patch for bug#42208Vladislav Vaintroub18 Feb
            • Re: Patch for bug#42208Ann W. Harrison18 Feb
              • RE: Patch for bug#42208Vladislav Vaintroub18 Feb
                • Re: Patch for bug#42208Jim Starkey18 Feb
                  • Re: Patch for bug#42208Ann W. Harrison18 Feb
                    • Re: Patch for bug#42208Ann W. Harrison18 Feb
                • Re: Patch for bug#42208Ann W. Harrison18 Feb
                  • RE: Patch for bug#42208Vladislav Vaintroub18 Feb
                    • Re: Patch for bug#42208Ann W. Harrison18 Feb
                    • Re: Patch for bug#42208Kevin Lewis18 Feb
                      • Re: Patch for bug#42208Ann W. Harrison18 Feb
      • Re: Patch for bug#42208MARK CALLAGHAN17 Feb
  • Re: Patch for bug#42208Jim Starkey16 Feb