Re: [Tarantool-patches] [PATCH] iproto: add an empty body to the unprepare response

[Nikita Pettik <korablev@tarantool.org>]

On 02 Mar 19:37, Chris Sosnin wrote:
> Yes, I was told that writing tests in python is kind of deprecated thing,
> however this test fits here quite well and makes patch much more concise.

Alternatively, you can introduce box/iproto.test.lua which will
be used for 'raw' iproto requests handling (e.g. to test raw iproto
requests containing new diagnostics area:
https://github.com/tarantool/tarantool/commit/5779ae4b562df18e4ec2e6c8821441d0d507f8f4#diff-af67795b74dc02a6ae1eab64b8fa920cR12).
As a reference you can look at gh-4077-iproto-execute-no-binds.test.lua

LGTM otherwise.
 
> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> index f313af6ae..d5891391d 100644
> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc
> @@ -1781,20 +1781,23 @@ tx_process_sql(struct cmsg *m)
>  	 */
>  	out = msg->connection->tx.p_obuf;
>  	struct obuf_svp header_svp;
> +	if (is_unprepare) {
> +		if (iproto_reply_ok(out, msg->header.sync, schema_version) != 0)
> +			goto error;
> +		iproto_wpos_create(&msg->wpos, out);
> +		return;
> +	}
>  	/* Prepare memory for the iproto header. */
>  	if (iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN) != 0) {
>  		port_destroy(&port);
>  		goto error;
>  	}
> -	/* Nothing to dump in case of UNPREPARE request. */
> -	if (!is_unprepare) {
> -		if (port_dump_msgpack(&port, out) != 0) {
> -			port_destroy(&port);
> -			obuf_rollback_to_svp(out, &header_svp);
> -			goto error;
> -		}
> +	if (port_dump_msgpack(&port, out) != 0) {
>  		port_destroy(&port);
> +		obuf_rollback_to_svp(out, &header_svp);
> +		goto error;
>  	}
> +	port_destroy(&port);
>  	iproto_reply_sql(out, &header_svp, msg->header.sync, schema_version);
>  	iproto_wpos_create(&msg->wpos, out);
>  	return;
> diff --git a/test/box-py/iproto.test.py b/test/box-py/iproto.test.py
> index 77637d8ed..101e5e913 100644
> --- a/test/box-py/iproto.test.py
> +++ b/test/box-py/iproto.test.py
> @@ -286,6 +286,20 @@ resp = test_request(header, body)
>  print 'Schema changed -> error:', resp['header'][0] != 0
>  print 'Got another schema_id:', resp['header'][5] != schema_id
>  
> +#
> +# gh-4769 Unprepare response must have a body.
> +#
> +IPROTO_PREPARE = 13
> +IPROTO_SQL_TEXT = 0x40
> +IPROTO_STMT_ID = 0x43
> +
> +header = { IPROTO_CODE : IPROTO_PREPARE }
> +body = { IPROTO_SQL_TEXT : 'SELECT 1' }
> +resp = test_request(header, body)
> +
> +body = { IPROTO_STMT_ID : resp['body'][IPROTO_STMT_ID] }
> +resp = test_request(header, body)
> +
>  #
>  # gh-2334 Lost SYNC in JOIN response.
>  #
> -- 
> 2.21.1 (Apple Git-122.3)
>