Re: [Tarantool-patches] [PATCH 1/2] vinyl: validate resulting tuple after upsert is applied

[Aleksandr Lyapunov <alyapunov@tarantool.org>]

Hi! thank again. See my comment below.

On 4/14/20 12:55 AM, Nikita Pettik wrote:
> diff --git a/src/box/vy_upsert.c b/src/box/vy_upsert.c
> index ebea2789c..6855b9820 100644
> --- a/src/box/vy_upsert.c
> +++ b/src/box/vy_upsert.c
> @@ -134,6 +134,10 @@ vy_apply_upsert(const struct tuple *new_stmt, const struct tuple *old_stmt,
>   					 &mp_size, 0, suppress_error,
>   					 &column_mask);
>   	result_mp_end = result_mp + mp_size;
> +	if (tuple_validate_raw(format, result_mp) != 0) {
> +		region_truncate(region, region_svp);
> +		return NULL;
> +	}
That is wrong for UPSERT-UPSERT case.
The result of applying an upsert to another must be such a cumulative
upsert that being applied to anything must have the same result as
two original upserts applied sequentially.
Also in any case, inapplicable ops must be skipped and inapplicable whole
upsert must be skipped.
Suppose we have two upserts:
upsert1 = {tuple1, {ops1}} and upsert2 = {tuple2, {ops2}},
and {ops2} are not applicable to tuple1.
What cumulative upsert must we create?
1)if apllied to NULL or DELETE first upsert must insert tuple1 and upsert2
must be skipped and thus the result must be tuple1.
2)if applied to some good tuple - all ops {ops1,ops2} must be applied.
Thus the cumulative upsert must be like:
{tuple1, {ops1, ops2}}
But in your code you create:
{tuple1, {ops1}}