[tarantool-patches] Re: [PATCH v3 5/6] sql: 'PRAGMA' result in the appropriate format

n.pettik korablev at tarantool.org
Wed Jan 16 18:35:25 MSK 2019


>> There is no ’Tarantool format’...
> Changed to "more appropriate”.

Cmon, it is called YAML...

> 
>> 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.
> Changed rows with values == NULL. Now instean of NULL they shows
> "No value”.

Now it is even worse.
I vote for displaying only flag pragmas and make
second field of resulting filed to be of type bool. 
You can ask other members for their opinion.

>> Btw I don’t understand all these efforts to make pragma
>> better. As I already said we are going to remove it anyway.
> I thought it would be correct, since it can be used by the user.

It can be used by users until we remove it. That’s it.

> commit 1b0d86990ecf8c9b310cdfdc58ec9006c77c038a
> Author: Mergen Imeev <imeevma at gmail.com>
> Date:   Thu Dec 13 21:07:31 2018 +0300
> 
>    sql: 'PRAGMA' result in the appropriate format
> 
>    Currently, box.sql.execute('PRAGMA') returns nothing but it prints
>    some information on stdout.

Nit: be specific: not ’some information’ but list of pragmas
and their statuses.

Nit: print to stdout.

> This looks wrong.

"Such strategy is considered to be wrong since output
of this command would be unavailable for users who
redirect stdout, use net box connection etc."

> This patch makes the
>    command to return result in more appropriate format.

"In yaml format as the rest of SQL commands.”

Sorry for being too meticulous.

> 
> diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
> index 692fa82..9c5c9b3 100644
> --- a/test/sql/sql-debug.result
> +++ b/test/sql/sql-debug.result

Why did you move test to this file?
PRAGMA itself is not debug-only command.






More information about the Tarantool-patches mailing list