Tarantool development patches archive
 help / color / mirror / Atom feed
From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Imeev Mergen <imeevma@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v2 5/6] sql: 'PRAGMA' result in Tarantool format
Date: Mon, 24 Dec 2018 16:02:10 +0200	[thread overview]
Message-ID: <53F2238B-909C-4ACB-A80D-9D271876B7A6@tarantool.org> (raw)
In-Reply-To: <78a21763e93af6b3e6977ff4d211371779954d0f.1544871594.git.imeevma@gmail.com>

There is no ’Tarantool format’...

> On 15 Dec 2018, at 14:05, imeevma@tarantool.org wrote:
> 
> Currently, box.sql.execute('PRAGMA') returns nothing but it prints
> some information on stdout. This isn't right.

Why do you consider it as wrong? AFAIR I made it exclusively to
extend debug facilities.

Things like
  - ['index_info', null]
  - ['index_list', null]

look quite strange IMO. What is more, it may be misleading:
index_list with null arg looks like there is no indexes at all.

Btw I don’t understand all these efforts to make pragma
better. As I already said we are going to remove it anyway.


> This patch makes
> the command to return result in Tarantool format.
> 
> Part of #3832
> ---
> src/box/sql/pragma.c   | 83 +++++++++++++++++++++++++++++---------------------
> test/sql/misc.result   | 12 ++++++++
> test/sql/misc.test.lua |  5 +++
> 3 files changed, 65 insertions(+), 35 deletions(-)
> 
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index 9390f6f..fc87976 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -168,48 +168,61 @@ pragmaLocate(const char *zName)
> 	return lwr > upr ? 0 : &aPragmaName[mid];
> }
> 
> -#ifdef PRINT_PRAGMA
> -#undef PRINT_PRAGMA
> -#endif
> -#define PRINT_PRAGMA(pragma_name, pragma_flag) do {			       \
> -	int nCoolSpaces = 30 - strlen(pragma_name);			       \
> -	if (user_session->sql_flags & (pragma_flag)) {			       \
> -		printf("%s %*c --  [true] \n", pragma_name, nCoolSpaces, ' '); \
> -	} else {							       \
> -		printf("%s %*c --  [false] \n", pragma_name, nCoolSpaces, ' ');\
> -	}								       \
> -} while (0)
> -
> -#define PRINT_STR_PRAGMA(pragma_name, str_value) do {			       \
> -	int nCoolSpaces = 30 - strlen(pragma_name);			       \
> -	printf("%s %*c --  '%s' \n", pragma_name, nCoolSpaces, ' ', str_value);\
> -} while (0)
> -
> static void
> -printActivePragmas(struct session *user_session)
> +sql_pragma_active_pragmas(struct Parse *parse)

Name seems to be a mess, lets rename it to ’sql_pragma_status’ or
‘vdbe_emit_pragma_status’.

> {
> -	int i;
> -	for (i = 0; i < ArraySize(aPragmaName); ++i) {
> +	struct Vdbe *v = sqlite3GetVdbe(parse);
> +	struct session *user_session = current_session();
> +
> +	sqlite3VdbeSetNumCols(v, 2);
> +	sqlite3VdbeSetColName(v, 0, COLNAME_NAME, "pragma_name", SQLITE_STATIC);
> +	sqlite3VdbeSetColName(v, 0, COLNAME_DECLTYPE, "TEXT", SQLITE_STATIC);
> +	sqlite3VdbeSetColName(v, 1, COLNAME_NAME, "pragma_value",
> +			      SQLITE_STATIC);
> +	sqlite3VdbeSetColName(v, 1, COLNAME_DECLTYPE, "TEXT", SQLITE_STATIC);
> +
> +	parse->nMem = 2;
> +	for (int i = 0; i < ArraySize(aPragmaName); ++i) {
> +		sqlite3VdbeAddOp4(v, OP_String8, 0, 1, 0, aPragmaName[i].zName,
> +				  0);
> 		switch (aPragmaName[i].ePragTyp) {
> -			case PragTyp_FLAG:
> -				PRINT_PRAGMA(aPragmaName[i].zName, aPragmaName[i].iArg);
> +			case PragTyp_PARSER_TRACE:
> +			case PragTyp_CASE_SENSITIVE_LIKE:
> +			case PragTyp_FLAG: {
> +				const char *value;
> +				if ((user_session->sql_flags &
> +				     aPragmaName[i].iArg) != 0)
> +					value = "true";
> +				else
> +					value = "false";
> +				sqlite3VdbeAddOp4(v, OP_String8, 0, 2, 0, value,
> +						  0);
> 				break;
> +			}
> 			case PragTyp_DEFAULT_ENGINE: {
> -				const char *engine_name =
> -					sql_storage_engine_strs[
> -						current_session()->
> -							sql_default_engine];
> -				PRINT_STR_PRAGMA(aPragmaName[i].zName,
> -						 engine_name);
> +				const char *value = sql_storage_engine_strs[
> +					user_session->sql_default_engine];
> +				sqlite3VdbeAddOp4(v, OP_String8, 0, 2, 0, value,
> +						  0);
> 				break;
> 			}
> +			case PragTyp_BUSY_TIMEOUT: {
> +				int value = parse->db->busyTimeout;
> +				sqlite3VdbeAddOp2(v, OP_Integer, value, 2);
> +				sqlite3VdbeAddOp2(v, OP_Cast, 2, AFFINITY_TEXT);
> +				break;
> +			}
> +			case PragTyp_COMPOUND_SELECT_LIMIT: {
> +				int value = sqlite3_limit(parse->db,
> +					SQL_LIMIT_COMPOUND_SELECT, -1);
> +				sqlite3VdbeAddOp2(v, OP_Integer, value, 2);
> +				sqlite3VdbeAddOp2(v, OP_Cast, 2, AFFINITY_TEXT);
> +				break;
> +			}
> +			default:
> +				sqlite3VdbeAddOp2(v, OP_Null, 0, 2);
> 		}
> -	}
> -
> -	printf("Other available pragmas: \n");
> -	for (i = 0; i < ArraySize(aPragmaName); ++i) {
> -		if (aPragmaName[i].ePragTyp != PragTyp_FLAG)
> -			printf("-- %s \n", aPragmaName[i].zName);
> +		sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 2);
> 	}
> }
> 
> @@ -440,7 +453,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
> 
> 	zLeft = sqlite3NameFromToken(db, pId);
> 	if (!zLeft) {
> -		printActivePragmas(user_session);
> +		sql_pragma_active_pragmas(pParse);
> 		return;
> 	}
> 
> diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
> index 5952035..1c0dd07 100644
> --- a/test/sql/misc.test.lua
> +++ b/test/sql/misc.test.lua
> @@ -24,3 +24,8 @@ result[1][1] == 0 or result[1][1] == 1
> -- Check that "PRAGMA case_sensitive_like" returns nothing if
> -- called with parameter.
> box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
> +
> +-- Make command "PRAGMA" return result in Tarantool format.
> +res = box.sql.execute('PRAGMA')
> +#res > 0
> +res[1][1]

This test look ridiculous: number of elements in res
doesn’t mean anything; first element with ‘busy_timeout’
value as well. I guess you are willing to avoid fixing
this test each time number/content of pragmes are changed.
And it this acceptable solution. On the other hand,
it barely tests anything. So I propose to display full
content of pragma.

  reply	other threads:[~2018-12-24 14:02 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-15 11:51 [tarantool-patches] [PATCH v2 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2018-12-15 11:54 ` [tarantool-patches] [PATCH v2 1/6] sql: remove unnecessary macros from pragma.* imeevma
2018-12-20 20:41   ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-24 14:01   ` n.pettik
2018-12-15 11:56 ` [tarantool-patches] [PATCH v2 2/6] sql: fix "PRAGMA parser_trace" result imeevma
2018-12-24 14:01   ` [tarantool-patches] " n.pettik
2018-12-15 12:01 ` [tarantool-patches] [PATCH v2 3/6] sql: Show currently set sql_default_engine imeevma
2018-12-24 14:01   ` [tarantool-patches] " n.pettik
2018-12-15 12:03 ` [tarantool-patches] [PATCH v2 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
2018-12-24 14:01   ` [tarantool-patches] " n.pettik
2018-12-15 12:05 ` [tarantool-patches] [PATCH v2 5/6] sql: 'PRAGMA' result in Tarantool format imeevma
2018-12-24 14:02   ` n.pettik [this message]
2018-12-15 12:08 ` [tarantool-patches] [PATCH v2 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2018-12-24 14:02   ` [tarantool-patches] " n.pettik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53F2238B-909C-4ACB-A80D-9D271876B7A6@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v2 5/6] sql: '\''PRAGMA'\'' result in Tarantool format' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox