fix parsing of GOOG128 type (len>=26)

[jsulmont]

Hi
Google user identifier seen at the moment on AdEx are 27 char.
I am not sure about potential side effects of this -- here's what Jeremy said when discussing the topic on Skype:

"there are some implications for us internally to change it, as it changes the value of hash() and we have files serialized with those hashes"

But I don't see why, since the hash should be computed on the internal 128 bits.

Cheers

[jeremybarnes]

The reason that it could cause a problem is that:

• Previously, if we had a 27 character Google ID, it would be stored as a string, and its hash would have been cityHashString(str).
• With this patch, it will be stored as a 128 bit integer, and the hash will be cityHash128(bits)

Those hashes will be different. Now imagine a memory mapped hash table that had been previously serialized under the old hashes. An attempt to search for it using the new hash will not find the entry, since it was under the previous hash.

That being said, this pull request should be merged; we will just need some time to manage the fallout before we do so.

[jsulmont]

Yes. Actually the hash lookup was fail because the internal types are different (STR vs GOOG128).
Since when is Google sending 27 character Google ID? I was under the impression that this type wasn't used ..

On Jul 2, 2013, at 6:33 PM, Jeremy Barnes notifications@github.com wrote:

The reason that it could cause a problem is that:

Previously, if we had a 27 character Google ID, it would be stored as a string, and its hash would have been cityHashString(str).
With this patch, it will be stored as a 128 bit integer, and the hash will be cityHash128(bits)
Those hashes will be different. Now imagine a memory mapped hash table that had been previously serialized under the old hashes. An attempt to search for it using the new hash will not find the entry, since it was under the previous hash.

That being said, this pull request should be merged; we will just need some time to manage the fallout before we do so.

—
Reply to this email directly or view it on GitHub.