[tarantool-patches] Re: [PATCH v2 5/6] sql: 'PRAGMA' result in Tarantool format
n.pettik
korablev at tarantool.org
Mon Dec 24 17:02:10 MSK 2018
There is no ’Tarantool format’...
> On 15 Dec 2018, at 14:05, imeevma at 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.
More information about the Tarantool-patches
mailing list