Re: [PATCH v5 2/4] memtx: introduce tuple compare hint

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Thu, Mar 07, 2019 at 12:44:06PM +0300, Kirill Shcherbatov wrote:
> Implement functions for retrieving tuple hints for a particular
> key_def. Hint is an integer that can be used for tuple comparison
> optimization: if a hint of one tuple is less than a hint of another
> then the first tuple is definitely less than the second; only if
> hints are equal tuple_compare must be called for getting comparison
> result.
> 
> Hints are calculated using only the first part of key_def.
> 
> Hints are only useful when:
>  * they are precalculated and stored along with the tuple;
> calculation is not cheap (cheaper than tuple_compare btw) but
> precalculated hints allow to compare tuples without even fetching
> tuple data.
>  * first part of key_def is 'string'(for now)
>  * since hint is calculated using only the first part of key_def
> (and only first several characters if it is a string) this part
> must be different with high probability for every tuple pair.
> 
> Closes #3961
> ---
>  src/box/key_def.c        |   1 +
>  src/box/key_def.h        | 119 ++++++++++++
>  src/box/memtx_tree.c     |  32 +++-
>  src/box/tuple_compare.cc | 381 +++++++++++++++++++++++++++++++++++++++
>  src/box/tuple_compare.h  |   7 +
>  src/lib/coll/coll.c      |  33 ++++
>  src/lib/coll/coll.h      |   4 +
>  7 files changed, 567 insertions(+), 10 deletions(-)
> 
> diff --git a/src/box/key_def.c b/src/box/key_def.c
> index d4c979aa1..771c30172 100644
> --- a/src/box/key_def.c
> +++ b/src/box/key_def.c
> @@ -136,6 +136,7 @@ key_def_set_func(struct key_def *def)
>  	key_def_set_compare_func(def);
>  	key_def_set_hash_func(def);
>  	key_def_set_extract_func(def);
> +	key_def_set_cmp_aux_func(def);

You can set the extra functions in key_def_set_compare_func, because
they are all defined in tuple_compare.cc.

>  }
>  
>  static void
> diff --git a/src/box/key_def.h b/src/box/key_def.h
> index dd62f6a35..c630e6b43 100644
> --- a/src/box/key_def.h
> +++ b/src/box/key_def.h
> @@ -115,6 +115,28 @@ struct key_part {
>  
>  struct key_def;
>  struct tuple;
> +/** Comparasion auxiliary information. */
> +typedef union {
> +	/**
> +	 * Hint is a value h(t) is calculated for tuple in terms
> +	 * of particular key_def that has follows rules:

s/follows/the following

> +	 * if h(t1) < h(t2) then t1 < t2;
> +	 * if h(t1) > h(t2) then t1 > t2;
> +	 * if t1 == t2 then h(t1) == h(t2);

The last statement isn't true. Please fix.

> +	 * These rules means that instead of direct tuple vs tuple

s/means/mean

> +	 * (or tuple vs key) comparison one may compare theirs
> +	 * hints first; and only if theirs hints are equal compare

s/are equal/equal

> +	 * the tuples themselves.
> +	 */
> +	uint64_t hint;
> +} cmp_aux_t;
> +
> +/** Test if cmp_aux_t a and b are equal. */
> +static inline bool
> +cmp_aux_equal(cmp_aux_t a, cmp_aux_t b)
> +{
> +	return a.hint == b.hint;
> +}

After having pondered the issue for a while, I tend to agree with
Kostja - we better call this auxiliary data 'hint' for both uni-
and multi- key indexes, because cmp_aux sounds awkward and looks
more like a function name, not a type name; it's easy to confuse
with tuple_aux_compare.

That being said, this is how I see it now:

 - There shouldn't be a special type for the auxiliary data. It should
   be of type uint64_t. The members/variables should be called 'hint'.
   Comments should be enough to draw the difference between speeding up
   comparisons and multikey indexing. It shouldn't make the code any
   more difficult to read, because an index implementation will not
   interpret hints anyhow - it will bluntly use 'hinted' key_def
   methods. All the interpretation will be done in a few chosen index
   methods (get, create_iterator replace, build_next index_vtab methods
   in case of memtx).

 - The new key_def virtual functions should be called

   tuple_compare_hinted
   tuple_compare_with_key_hinted
   tuple_hint
   key_hint

   They should all operate with uint64_t hint, the meaning of which will
   depend on the index implementation.

 - We must not define tuple_compare and tuple_compare_with_key as well
   as plain key extractors for multikey indexes (corresponding members
   should be set to NULLs in key_def struct), because they simply don't
   make any sense for them. We must not define key_hint and tuple_hint
   methods either, because those need extra info (multikey offset).
   Passing an offset to those methods explicitly, like you do in the
   next patch, doesn't look good. Instead we should set hints in index
   vtab methods directly, without the use of key_hint/tuple_hint virtual
   methods.

>  
>  /**
>   * Get is_nullable property of key_part.
> @@ -137,6 +159,18 @@ typedef int (*tuple_compare_with_key_t)(const struct tuple *tuple_a,
>  typedef int (*tuple_compare_t)(const struct tuple *tuple_a,
>  			       const struct tuple *tuple_b,
>  			       struct key_def *key_def);
> +/** @copydoc tuple_aux_compare_with_key() */
> +typedef int (*tuple_aux_compare_with_key_t)(const struct tuple *tuple,
> +					    cmp_aux_t tuple_cmp_aux,
> +					    const char *key, uint32_t part_count,
> +					    cmp_aux_t key_cmp_aux,
> +					    struct key_def *key_def);
> +/** @copydoc tuple_aux_compare() */
> +typedef int (*tuple_aux_compare_t)(const struct tuple *tuple_a,
> +				   cmp_aux_t tuple_a_cmp_aux,
> +				   const struct tuple *tuple_b,
> +				   cmp_aux_t tuple_b_cmp_aux,
> +				   struct key_def *key_def);
>  /** @copydoc tuple_extract_key() */
>  typedef char *(*tuple_extract_key_t)(const struct tuple *tuple,
>  				     struct key_def *key_def,
> @@ -153,12 +187,23 @@ typedef uint32_t (*tuple_hash_t)(const struct tuple *tuple,
>  typedef uint32_t (*key_hash_t)(const char *key,
>  				struct key_def *key_def);
>  
> +/** @copydoc tuple_cmp_aux() */
> +typedef cmp_aux_t (*tuple_cmp_aux_t)(const struct tuple *tuple,
> +				     struct key_def *key_def);
> +
> +/** @copydoc key_cmp_aux() */
> +typedef cmp_aux_t (*key_cmp_aux_t)(const char *key, struct key_def *key_def);
> +
>  /* Definition of a multipart key. */
>  struct key_def {
>  	/** @see tuple_compare() */
>  	tuple_compare_t tuple_compare;
>  	/** @see tuple_compare_with_key() */
>  	tuple_compare_with_key_t tuple_compare_with_key;
> +	/** @see tuple_aux_compare_with_key() */
> +	tuple_aux_compare_with_key_t tuple_aux_compare_with_key;
> +	/** @see tuple_aux_compare() */
> +	tuple_aux_compare_t tuple_aux_compare;
>  	/** @see tuple_extract_key() */
>  	tuple_extract_key_t tuple_extract_key;
>  	/** @see tuple_extract_key_raw() */
> @@ -167,6 +212,10 @@ struct key_def {
>  	tuple_hash_t tuple_hash;
>  	/** @see key_hash() */
>  	key_hash_t key_hash;
> +	/** @see tuple_cmp_aux() */
> +	tuple_cmp_aux_t tuple_cmp_aux;
> +	/** @see key_cmp_aux() */
> +	key_cmp_aux_t key_cmp_aux;
>  	/**
>  	 * Minimal part count which always is unique. For example,
>  	 * if a secondary index is unique, then
> @@ -571,6 +620,52 @@ tuple_compare_with_key(const struct tuple *tuple, const char *key,
>  	return key_def->tuple_compare_with_key(tuple, key, part_count, key_def);
>  }
>  
> +/**
> + * Compare tuples using the key definition and comparasion
> + * auxillary information.

Auxillary

> + * @param tuple_a First tuple.
> + * @param tuple_a_cmp_aux Comparasion auxiliary information

Comparasion

> + *                        for the tuple_a.
> + * @param tuple_b Second tuple.
> + * @param tuple_b_cmp_aux Comparasion auxilary information

auxilary

Please enable spell checking already.

> diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
> index cf4519ccb..5b06e06b3 100644
> --- a/src/box/tuple_compare.cc
> +++ b/src/box/tuple_compare.cc
> @@ -1323,9 +1323,390 @@ tuple_compare_with_key_create(const struct key_def *def)
>  
>  /* }}} tuple_compare_with_key */
>  
> +/* {{{ tuple_aux_compare */
> +
> +#define CMP_HINT_INVALID ((uint64_t)UINT64_MAX)

Pointless type cast.

> +
> +static int
> +tuple_hinted_compare(const struct tuple *tuple_a, cmp_aux_t tuple_a_cmp_aux,
> +		     const struct tuple *tuple_b, cmp_aux_t tuple_b_cmp_aux,
> +		     struct key_def *key_def)
> +{
> +	uint64_t tuple_a_hint = tuple_a_cmp_aux.hint;
> +	uint64_t tuple_b_hint = tuple_b_cmp_aux.hint;
> +	if (likely(tuple_a_hint != tuple_b_hint &&
> +		   tuple_a_hint != CMP_HINT_INVALID &&
> +		   tuple_b_hint != CMP_HINT_INVALID))
> +		return tuple_a_hint < tuple_b_hint ? -1 : 1;
> +	else
> +		return tuple_compare(tuple_a, tuple_b, key_def);
> +}
> +
> +static tuple_aux_compare_t
> +tuple_aux_compare_create(const struct key_def *def)
> +{
> +	(void)def;
> +	return tuple_hinted_compare;
> +}
> +
> +/* }}} tuple_aux_compare */
> +
> +/* {{{ tuple_aux_compare_with_key */
> +
> +static int
> +tuple_hinted_compare_with_key(const struct tuple *tuple, cmp_aux_t tuple_cmp_aux,
> +			      const char *key, uint32_t part_count,
> +			      cmp_aux_t key_cmp_aux, struct key_def *key_def)
> +{
> +	uint64_t tuple_hint = tuple_cmp_aux.hint;
> +	uint64_t key_hint = key_cmp_aux.hint;
> +	if (likely(tuple_hint != key_hint && tuple_hint != CMP_HINT_INVALID &&

A sane compiler will treat this branch as likely anyways, because the
other branch involves an indirect function call.

> +		   key_hint != CMP_HINT_INVALID))
> +		return tuple_hint < key_hint ? -1 : 1;
> +	else
> +		return tuple_compare_with_key(tuple, key, part_count, key_def);
> +}
> +
> +static tuple_aux_compare_with_key_t
> +tuple_aux_compare_with_key_create(const struct key_def *def)
> +{
> +	(void)def;
> +	return tuple_hinted_compare_with_key;
> +}
> +
> +/* }}} tuple_aux_compare_with_key */
> +
>  void
>  key_def_set_compare_func(struct key_def *def)
>  {
>  	def->tuple_compare = tuple_compare_create(def);
>  	def->tuple_compare_with_key = tuple_compare_with_key_create(def);
> +	def->tuple_aux_compare = tuple_aux_compare_create(def);
> +	def->tuple_aux_compare_with_key =
> +		tuple_aux_compare_with_key_create(def);
> +}
> +
> +/* Tuple hints */
> +
> +static cmp_aux_t
> +key_hint_default(const char *key, struct key_def *key_def)
> +{
> +	(void)key;
> +	(void)key_def;
> +	cmp_aux_t ret;
> +	ret.hint = CMP_HINT_INVALID;
> +	return ret;
> +}
> +
> +static cmp_aux_t
> +tuple_hint_default(const struct tuple *tuple, struct key_def *key_def)
> +{
> +	(void)tuple;
> +	(void)key_def;
> +	cmp_aux_t ret;
> +	ret.hint = CMP_HINT_INVALID;
> +	return ret;
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +key_hint_uint(const char *key, struct key_def *key_def)
> +{
> +	(void)key_def;
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_UNSIGNED);
> +	cmp_aux_t ret;
> +	if (key == NULL || (is_nullable && mp_typeof(*key) == MP_NIL)) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	assert(mp_typeof(*key) == MP_UINT);
> +	uint64_t val = mp_decode_uint(&key);
> +	ret.hint = unlikely(val > INT64_MAX) ? INT64_MAX :
> +		   val - (uint64_t)INT64_MIN;;

Double semicolon (;) at the end of the string.

> +	return ret;
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +tuple_hint_uint(const struct tuple *tuple, struct key_def *key_def)
> +{
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_UNSIGNED);
> +	cmp_aux_t ret;
> +	const char *field = tuple_field_by_part(tuple, key_def->parts);
> +	if (is_nullable && field == NULL) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	return key_hint_uint<is_nullable>(field, key_def);
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +key_hint_int(const char *key, struct key_def *key_def)
> +{
> +	(void)key_def;
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_INTEGER);
> +	cmp_aux_t ret;
> +	if (key == NULL || (is_nullable && mp_typeof(*key) == MP_NIL)) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	if (mp_typeof(*key) == MP_UINT) {
> +		uint64_t val = mp_decode_uint(&key);
> +		ret.hint = unlikely(val > INT64_MAX) ? INT64_MAX :
> +			   val - (uint64_t)INT64_MIN;

Both branches are equally cheap so 'unlikely' is pointless.

Please stop using likely/unlikely without a good reason.

> +	} else {
> +		assert(mp_typeof(*key) == MP_INT);
> +		int64_t val = mp_decode_int(&key);
> +		ret.hint = (uint64_t)val - (uint64_t)INT64_MIN;
> +	}
> +	return ret;
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +tuple_hint_int(const struct tuple *tuple, struct key_def *key_def)
> +{
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_INTEGER);
> +	cmp_aux_t ret;
> +	const char *field = tuple_field_by_part(tuple, key_def->parts);
> +	if (is_nullable && field == NULL) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	return key_hint_int<is_nullable>(field, key_def);
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +key_hint_number(const char *key, struct key_def *key_def)
> +{
> +	(void)key_def;
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_NUMBER);
> +	cmp_aux_t ret;
> +	if (key == NULL || (is_nullable && mp_typeof(*key) == MP_NIL)) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	uint64_t val = 0;
> +	switch (mp_typeof(*key)) {
> +	case MP_FLOAT:
> +	case MP_DOUBLE: {

A comment explaining what's going on here would be useful.

> +		double f = mp_typeof(*key) == MP_FLOAT ?
> +			   mp_decode_float(&key) : mp_decode_double(&key);
> +		if (isnan(f) || isinf(f)) {

if (!isfinite(f))

> +			ret.hint = CMP_HINT_INVALID;
> +			return ret;
> +		}
> +		double ival;
> +		(void)modf(f, &ival);
> +		if (unlikely(ival >= (double)INT64_MAX)) {

Again, 'unlikely' is pointless. Better remove all likely/unlikely from
the patch.

> +			ret.hint = INT64_MAX;
> +			return ret;
> +		}
> +		if (unlikely(ival <= (double)INT64_MIN)) {
> +			ret.hint = 0;
> +			return ret;
> +		}
> +		val = (uint64_t)ival;

This check isn't quite correct. Try running the following code:


	#include <stdio.h>
	#include <stdint.h>

	int main()
	{
		double val = INT64_MAX;
		printf("val is %lf\n", val);
		printf("val <= (double)INT64_MAX is %s\n",
		       val <= (double)INT64_MAX ? "true" : "false");
		printf("(int64_t)val is %lld\n", (int64_t)val);
	}

Here's what I get:

	val is 9223372036854775808.000000
	val <= (double)INT64_MAX is true
	(int64_t)val is -9223372036854775808

> +		break;
> +	}
> +	case MP_INT: {
> +		val = (uint64_t)mp_decode_int(&key);
> +		break;
> +	}
> +	case MP_UINT: {
> +		val = mp_decode_uint(&key);
> +		if (val > INT64_MAX) {
> +			ret.hint = INT64_MAX;
> +			return ret;
> +		}
> +		break;
> +	}
> +	default:
> +		unreachable();
> +	}
> +	ret.hint = val - (uint64_t)INT64_MIN;
> +	return ret;
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +tuple_hint_number(const struct tuple *tuple, struct key_def *key_def)
> +{
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_NUMBER);
> +	cmp_aux_t ret;
> +	const char *field = tuple_field_by_part(tuple, key_def->parts);
> +	if (is_nullable && field == NULL) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	return key_hint_number<is_nullable>(field, key_def);
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +key_hint_boolean(const char *key, struct key_def *key_def)
> +{
> +	(void)key_def;
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_BOOLEAN);
> +	cmp_aux_t ret;
> +	if (key == NULL || (is_nullable && mp_typeof(*key) == MP_NIL)) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	bool val = mp_decode_bool(&key);
> +	ret.hint = (uint64_t)val - (uint64_t)INT64_MIN;

What's this for?

> +	return ret;
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +tuple_hint_boolean(const struct tuple *tuple, struct key_def *key_def)
> +{
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_BOOLEAN);
> +	const char *field = tuple_field_by_part(tuple, key_def->parts);
> +	cmp_aux_t ret;
> +	if (is_nullable && field == NULL) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	return key_hint_boolean<is_nullable>(field, key_def);
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +key_hint_string(const char *key, struct key_def *key_def)
> +{
> +	(void)key_def;
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->coll == NULL);
> +	assert(key_def->parts->type == FIELD_TYPE_STRING);
> +	cmp_aux_t ret;
> +	if (key == NULL || (is_nullable && mp_typeof(*key) == MP_NIL)) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	assert(mp_typeof(*key) == MP_STR);
> +	uint32_t len;
> +	const unsigned char *str =
> +		(const unsigned char *)mp_decode_str(&key, &len);
> +	uint64_t result = 0;
> +	uint32_t process_len = MIN(len, 8);
> +	for (uint32_t i = 0; i < process_len; i++) {
> +		result <<= 8;
> +		result |= str[i];
> +	}
> +	result <<= 8 * (8 - process_len);
> +	ret.hint = result;
> +	return ret;
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +tuple_hint_string(const struct tuple *tuple, struct key_def *key_def)
> +{
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->coll == NULL);
> +	assert(key_def->parts->type == FIELD_TYPE_STRING);
> +	cmp_aux_t ret;
> +	const char *field = tuple_field_by_part(tuple, key_def->parts);
> +	if (is_nullable && field == NULL) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	return key_hint_string<is_nullable>(field, key_def);
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +key_hint_string_coll(const char *key, struct key_def *key_def)
> +{
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_STRING &&
> +	        key_def->parts->coll != NULL);
> +	cmp_aux_t ret;
> +	if (key == NULL || (is_nullable && mp_typeof(*key) == MP_NIL)) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	assert(mp_typeof(*key) == MP_STR);
> +	uint32_t len;
> +	const char *str = mp_decode_str(&key, &len);
> +	ret.hint = key_def->parts->coll->hint(str, len, key_def->parts->coll);
> +	return ret;
> +}
> +
> +template<bool is_nullable>
> +static cmp_aux_t
> +tuple_hint_string_coll(const struct tuple *tuple, struct key_def *key_def)
> +{
> +	assert(key_part_is_nullable(key_def->parts) == is_nullable);
> +	assert(key_def->parts->type == FIELD_TYPE_STRING &&
> +	        key_def->parts->coll != NULL);
> +	const char *field = tuple_field_by_part(tuple, key_def->parts);
> +	cmp_aux_t ret;
> +	if (is_nullable && field == NULL) {
> +		ret.hint = 0;
> +		return ret;
> +	}
> +	return key_hint_string_coll<is_nullable>(field, key_def);
> +}
> +
> +void
> +key_def_set_cmp_aux_func(struct key_def *def)
> +{
> +	def->key_cmp_aux = key_hint_default;
> +	def->tuple_cmp_aux = tuple_hint_default;
> +	bool is_nullable = key_part_is_nullable(def->parts);

> +	if (def->parts->type == FIELD_TYPE_STRING && def->parts->coll != NULL) {
> +		def->key_cmp_aux = is_nullable ? key_hint_string_coll<true> :
> +						 key_hint_string_coll<false>;
> +		def->tuple_cmp_aux = is_nullable ?
> +				     tuple_hint_string_coll<true> :
> +				     tuple_hint_string_coll<false>;
> +		return;
> +	}

Please move this to the switch-case below.

> +	switch (def->parts->type) {
> +	case FIELD_TYPE_UNSIGNED:
> +		def->key_cmp_aux = is_nullable ? key_hint_uint<true> :
> +						 key_hint_uint<false>;
> +		def->tuple_cmp_aux = is_nullable ? tuple_hint_uint<true> :
> +						   tuple_hint_uint<false>;
> +		break;
> +	case FIELD_TYPE_INTEGER:
> +		def->key_cmp_aux = is_nullable ? key_hint_int<true> :
> +						 key_hint_int<false>;
> +		def->tuple_cmp_aux = is_nullable ? tuple_hint_int<true> :
> +						   tuple_hint_int<false>;
> +		break;
> +	case FIELD_TYPE_STRING:
> +		def->key_cmp_aux = is_nullable ? key_hint_string<true> :
> +						 key_hint_string<false>;
> +		def->tuple_cmp_aux = is_nullable ? tuple_hint_string<true> :
> +						   tuple_hint_string<false>;
> +		break;
> +	case FIELD_TYPE_NUMBER:
> +		def->key_cmp_aux = is_nullable ? key_hint_number<true> :
> +						 key_hint_number<false>;
> +		def->tuple_cmp_aux = is_nullable ? tuple_hint_number<true> :
> +						   tuple_hint_number<false>;
> +		break;
> +	case FIELD_TYPE_BOOLEAN:
> +		def->key_cmp_aux = is_nullable ? key_hint_boolean<true> :
> +						 key_hint_boolean<false>;
> +		def->tuple_cmp_aux = is_nullable ? tuple_hint_boolean<true> :
> +						   tuple_hint_boolean<false>;
> +		break;
> +	default:
> +		break;
> +	};
>  }