Re: Patch for bug#42208

[Jim Starkey]

Vladislav Vaintroub wrote:
> 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.
>   
Wrong!  You need to bump the minor version number and create new indexes 
using a new index version number.  This means:

    * Old versions of Falcon will error on opening new databases.
    * New versions of Falcon will honor the previous index formats

When a new version of the software opens an old database, it should bump 
the minor version number (it only really needs to do this if it creates 
a new index).

Old indexes should remain valid and supported.
> 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 :)
>
>
>   
As per previous message, I don't think adding additional zero bytes to 
non-zero length keys does anything worthwhile.

-- 
Jim Starkey
President, NimbusDB, Inc.
978 526-1376