[tarantool-patches] Re: [PATCH v3 2/6] sql: fix "PRAGMA parser_trace" result

["n.pettik" <korablev@tarantool.org>]

> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index b8edc76..6122986 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -466,33 +466,31 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
> 	switch (pPragma->ePragTyp) {
> 
> 	case PragTyp_FLAG:{
> -			if (zRight == 0) {
> -				setPragmaResultColumnNames(v, pPragma);
> -				returnSingleInt(v,
> -						(user_session->
> -						 sql_flags & pPragma->iArg) !=
> -						0);
> -			} else {
> -				int mask = pPragma->iArg;	/* Mask of bits to set
> -								 * or clear.
> -								 */
> -
> -				if (sqlite3GetBoolean(zRight, 0)) {
> -					user_session->sql_flags |= mask;
> -				} else {
> -					user_session->sql_flags &= ~mask;
> -				}
> -
> -				/* Many of the flag-pragmas modify the code
> -				 * generated by the SQL * compiler (eg.
> -				 * count_changes). So add an opcode to expire
> -				 * all * compiled SQL statements after
> -				 * modifying a pragma value.
> -				 */
> -				sqlite3VdbeAddOp0(v, OP_Expire);
> +		if (zRight == 0) {

Since you have already aligned code, I propose to fix it
according to our codestyle:

zRight == NULL

> +			setPragmaResultColumnNames(v, pPragma);
> +			returnSingleInt(v, (user_session->sql_flags &
> +					    pPragma->iArg) != 0);
> +		} else {
> +			int mask = pPragma->iArg;	/* Mask of bits to set
> +							 * or clear.
> +							 */

Smth wrong with comment.

> +			bool is_value_true = sqlite3GetBoolean(zRight, 0);

Bad name imho. I would call it “is_set” or “is_pragma_set” or “pragma_status”.
Explanation: variable itself is of type bool and “value” says nothing.

> diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
> new file mode 100644
> index 0000000..66f47b3
> --- /dev/null
> +++ b/test/sql/sql-debug.test.lua
> @@ -0,0 +1,17 @@
> +remote = require('net.box')
> +test_run = require('test_run').new()
> +engine = test_run:get_cfg('engine')
> +box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
> +
> +--
> +-- gh-3832: Some statements do not return column type
> +
> +-- Check that "PRAGMA parser_trace" returns 0 or 1 if called
> +-- without parameter.
> +result = box.sql.execute('PRAGMA parser_trace')
> +-- Should be nothing.
> +box.sql.execute('PRAGMA parser_trace = 1')
> +-- Should be 1.
> +box.sql.execute('PRAGMA parser_trace')
> +-- Should be nothing.

These three comments say nothing.
Results and mismatches can be seen from .result and
.reject files.