Vindex varchar support

[ashudeep-sharma-zz]

This Pull Request contains the following change:

• Adding Vindex support for varchar and varbinary Column Type.

---

This change is

[sougou]

I don't think gob hashes the input. You can use this function instead: https://github.com/youtube/vitess/blob/master/go/vt/vtgate/vindexes/hash.go#L118. Should look something like this:

func binHash(key interface{}) ([]byte, error) {
  source, ok := key.([]byte)
  if !ok {
    return nil, fmt.Errorf("unexpected data type for binHash: %T", key)
  }
  dest := make([]byte, len(source))
  block3DES.Encrypt(dest, source)
  return dest, nil
}

[ashudeep-sharma-zz]

Review status: 0 of 4 files reviewed at latest revision, 2 unresolved discussions.

---

go/vt/vtgate/vindexes/varchar_hash.go, line 42 [r3] (raw file):
Made the changes to convert the data into lowercase before hashing. I have added a test-case with UpperCase,LowerCase and mixedCase letters to validate it. Let me know if anything else needs to be added.

---

go/vt/vtgate/vindexes/varchar_hash.go, line 48 [r3] (raw file):
created the binHash Function which gets the bytes, convert it into lowercase and then apply block3DES.Encrypt().

---

Comments from Reviewable

[sougou]

go/vt/vtgate/vindexes/varchar_hash.go, line 54 [r5] (raw file):
I don't think this will work as expected, because gob encodes the type info into the bytes. For example, int(1) and uint(1) will encode differently even though they're the same value. So, it's better if we asserted the value into the specific types we want to allow (like I showed in the binHash example). For now, I think allowing only []byte is enough. If we want to allow more types, we can extend it.

---

Comments from Reviewable

[sougou]

Reviewed 2 of 4 files at r4.
Review status: 2 of 4 files reviewed at latest revision, 1 unresolved discussion.

---

go/vt/vtgate/vindexes/varchar_hash.go, line 54 [r5] (raw file):
Sorry, my previous comment got sent out prematurely.

So, I think I confused you with my original comment. Basically, the binHash implementation I gave should have just replaced GetBytes. Everything else was fine. The older getHash function for varchar that performed the lower-casing was also fine, except that it would work only if you lower-cased before hashing.

---

Comments from Reviewable

[ashudeep-sharma-zz]

Review status: 0 of 4 files reviewed at latest revision, 2 unresolved discussions.

---

go/vt/vtgate/vindexes/utf8cihash.go, line 54 [r5] (raw file):
I have made the changes as per your suggestions, the binhash function now accepts a byte array and all the additional checks for normalization ha been moved to external functions

---

go/vt/vtgate/vindexes/varchar_hash.go, line 42 [r3] (raw file):
Done.

---

Comments from Reviewable

[sougou]

LGTM
I'll look at how to make it utf8 collation compliant after the submit.

---

Reviewed 6 of 6 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[sougou]

I've been doing some reading and testing about unicode collations. The good news is that I've found the perfect Go library to perform the correct normalization. It produces a normalized 'key' that is the lexical equivalent of the string.
The bad news is that the 3DES hashing function doesn't work as expected. It basically works in 64-bit blocks. Each call to block3DES.Encrypt only encrypts the first 64 bits. It actually crashes for short strings like "T". So, I have the following options:

1. Produce a 64-bit keyspace id using the first 4 characters of the input; 4 bytes of input produces 8 bytes (64-bits) of normalized key.
2. Prefix the string with the hash of the first 4 characters.
3. Hash the entire input by dividing it into 64-bit chunks.

I'll provide an initial implementation using option 1. If that's insufficient, we can look at the other options.

[enisoc]

Option 1 seems likely to bite us down the road. It may work as a custom vindex plugin for a given user, but it shouldn't be shipped along with Vitess core in that case because it has misleading properties. I could imagine wanting to shard on a varchar that often starts with the same 4 characters, for example an app-specific entity ID string like "uid-1234".

For option 3, how would you combine the resulting encrypted chunks? Would you just concatenate and have variable-length keyspace IDs?

Can we just use an irreversible fixed-width hash instead, since normalization is not reversible anyway?

[sougou]

Hmm. Fixed prefix would cause problems even for option 3 because all keys will start with the hash of the first 4 characters.
We'll have to go for stronger encryption, which won't be cheap. Or the simpler solution will be to not hash at all, instead go for non-uniform sharding. I'm leaning towards the latter.
@ashudeep-sharma what do you think?

[enisoc]

What about option 4: checksum the whole normalized key with something like MD5 or SHA-N?

[sougou]

MD5 looks good. I also looked up CRC, which they say is more efficient, but less random.
If people can put up with non-uniform sharding, some benefit can be derived from the unicode ordering. I'm leaning towards MD5, while we wait for @ashudeep-sharma's response.

[sougou]

Since vindex behavior should never be changed (once people start using it), I'm going to rename these to more accurately represent what they do:
varbinaryHash -> binaryMD5, vindex name: "binary_md5".
UTF8ciHash -> UnicodeLooseMD5, vindex name: "unicode_loose_md5"
The Loose term was coined by Go to indicate the most permissive matching. The technically correct term would be L1 (http://www.unicode.org/reports/tr10/#Multi_Level_Comparison), but it may be harder to explain.
Open to opinions, suggestions.

[ashudeep-sharma-zz]

The first option would definitely be a problem for use-cases where-in teams use some prefix, in our case lets say a tracking-id always starts from "FMCP".I tried searching for some other options but i think MD5 looks more promising.

[ashudeep-sharma-zz]

In my earlier companies, we have used Jenkins Hash which ensures evenness in distribution, but came across this post, it seems like it is not that optimized in GO.
http://stackoverflow.com/questions/23436358/bob-jenkins-hash-getting-bad-performance

[sougou]

I'm working on the change. You can see progress here https://github.com/sougou/vitess/commits/v3.
I'll send the PR out once I resolve the []byte vs string issue @ashudeep-sharma encountered earlier today.