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

[Chris Sosnin <k.sosnin@tarantool.org>]

Hi! Thank you for the review!

> 2 марта 2020 г., в 22:18, 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

This seems reasonable. Thanks, I will rework it and send a new version.


> 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)
>>