Re: [Tarantool-patches] [PATCH v5 4/8] refactoring: set diagnostics if sequence_by_id fails

[Sergey Ostanevich <sergos@tarantool.org>]

Hi!

LGTM. 

Please, create a follow-up tracker to review all sequence_by_id() uses:
for example, in on_drop_sequence_data_rollback() this can lead to a
coredump inside the sequence_set() in release build. 

Regards,
Sergos

On 22 Nov 05:46, Ilya Kosarev wrote:
> In refactoring: use non _xc version of functions in triggers
> (b75d5f85338158eb1734ca8832d3c4b8ac0b4086) sequence_cache_find was
> replaced by sequence_by_id. It led to the loss of diagnostics in case
> of sequence_by_id failure. Now it is fixed.
> 
> Part of #4247
> ---
>  src/box/alter.cc | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index aaaf53493..2628e1401 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -4365,8 +4365,10 @@ on_replace_dd_sequence_data(struct trigger * /* trigger */, void *event)
>  			    &id) != 0)
>  		return -1;
>  	struct sequence *seq = sequence_by_id(id);
> -	if (seq == NULL)
> +	if (seq == NULL) {
> +		diag_set(ClientError, ER_NO_SUCH_SEQUENCE, int2str(id));
>  		return -1;
> +	}
>  	if (new_tuple != NULL) {			/* INSERT, UPDATE */
>  		int64_t value;
>  		if (tuple_field_i64(new_tuple, BOX_SEQUENCE_DATA_FIELD_VALUE,
> @@ -4520,8 +4522,10 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
>  	if (space == NULL)
>  		return -1;
>  	struct sequence *seq = sequence_by_id(sequence_id);
> -	if (seq == NULL)
> +	if (seq == NULL) {
> +		diag_set(ClientError, ER_NO_SUCH_SEQUENCE, int2str(sequence_id));
>  		return -1;
> +	}
>  
>  	enum priv_type priv_type = stmt->new_tuple ? PRIV_C : PRIV_D;
>  	if (stmt->new_tuple && stmt->old_tuple)
> -- 
> 2.17.1
>