[tarantool-patches] Re: [PATCH v2 4/5] sql: encode tuples with mpstream on Vdbe run

[n.pettik]

> On 29 Dec 2018, at 12:49, Kirill Shcherbatov <kshcherbatov at tarantool.org> wrote:
> 
> Introduced new sql_vdbe_mem_encode_tuple and
> mpstream_encode_vdbe_mem routines to perform Vdbe memory to
> msgpack encoding on region without previous size estimation call.
> Got rid off sqlite3VdbeMsgpackRecordLen and

Nit: got rid of.

> +/**
> + * Perform encoding field_count Vdbe memory fields on region as
> + * msgpack array.
> + * @param fields The first Vdbe memory field to encode.
> + * @param field_count Count of fields to encode.
> + * @param[out] tuple_size Size of encoded tuple.
> + * @param region Region to use.
> + * @retval NULL on error, diag message is set.
> + * @retval no NULL tuple pointer on success.

Nit: not NULL … or simply “Pointer to valid tuple."

> @@ -1714,3 +1716,71 @@ sqlite3ValueBytes(sqlite3_value * pVal)
> 		return 0;
> 	return valueBytes(pVal);
> }
> +
> +/**
> + * Perform encoding memory variable to stream.
> + * @param stream Initialized mpstream encoder object.
> + * @param var Vdbe memory variable to encode with stream.
> + */
> +static void
> +mpstream_encode_vdbe_mem(struct mpstream *stream, struct Mem *var)
> +{
> +	assert(memIsValid(var));
> +	if (var->flags & MEM_Null) {
> +		mpstream_encode_nil(stream);
> +	} else if (var->flags & MEM_Real) {
> +		mpstream_encode_double(stream, var->u.r);
> +	} else if (var->flags & MEM_Int) {
> +		if (var->u.i >= 0)
> +			mpstream_encode_uint(stream, var->u.i);
> +		else
> +			mpstream_encode_int(stream, var->u.i);
> +	} else if (var->flags & MEM_Str) {
> +		mpstream_encode_strn(stream, var->z, var->n);
> +	} else if (var->flags & MEM_Bool) {
> +		mpstream_encode_bool(stream, var->u.b);
> +	} else {
> +		/*
> +		 * Emit BIN header iff the BLOB doesn't store
> +		 * MsgPack content.
> +		 */
> +		if ((var->flags & MEM_Subtype) == 0 ||
> +		     var->subtype != SQL_SUBTYPE_MSGPACK) {
> +			uint32_t binl = var->n +
> +					((var->flags & MEM_Zero) ?
> +					var->u.nZero : 0);
> +			mpstream_encode_binl(stream, binl);
> +		}
> +		mpstream_memcpy(stream, var->z, var->n);
> +		if (var->flags & MEM_Zero)
> +			mpstream_memset(stream, 0, var->u.nZero);
> +	}

Lets replace these if-else's with one switch.

The rest is OK.