Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org, Nikita Pettik <korablev@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH 4/6] sql: don't add string of 'CREATE TABLE...' to space opts
Date: Mon, 10 Dec 2018 17:17:46 +0300	[thread overview]
Message-ID: <6965549c-a919-0460-2603-b4765a32e338@tarantool.org> (raw)
In-Reply-To: <8d774a673688db0503e67b05be1ece319bfd9ea4.1544387419.git.korablev@tarantool.org>

Thanks for the patch! See 1 comment and review fixes below.

On 10/12/2018 00:30, Nikita Pettik wrote:
> We don't rely no more on this string anymore and it can be removed for

'no more on this anymore' - please, rephrase. Sounds a little
tautological.

> ordinary tables. However, it is still used to hold SELECT body for view.
> 
> Part of #2647
> ---
>   src/box/sql.c           |   8 +-
>   src/box/sql/analyze.c   |   2 +-
>   src/box/sql/build.c     | 242 ++----------------------------------------------
>   src/box/sql/pragma.c    |   2 -
>   src/box/sql/sqliteInt.h |   1 -
>   test/sql/upgrade.result |   8 +-
>   test/sql/view.result    |   3 +
>   test/sql/view.test.lua  |   1 +
>   8 files changed, 19 insertions(+), 248 deletions(-)
> 

My review fix here and on the branch:

====================================================================

commit e4ea3a20c71e73426a9ea2478b55c346b0ca3a41
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Mon Dec 10 16:57:18 2018 +0300

     Review fix

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 325c272b0..ce1f4aed7 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -294,7 +294,7 @@ sql_pragma_table_info(struct Parse *parse, const char *tbl_name)
  static int
  sql_pragma_table_stats(struct space *space, void *data)
  {
-	if (space->def->opts.sql == NULL || space->def->opts.is_view)
+	if (space->def->opts.is_view)
  		return 0;
  	struct Parse *parse = (struct Parse *) data;
  	struct index *pk = space_index(space, 0);

  reply	other threads:[~2018-12-10 14:17 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-09 21:30 [tarantool-patches] [PATCH 0/6] Remove string of SQL statement from opts Nikita Pettik
2018-12-09 21:30 ` [tarantool-patches] [PATCH 1/6] sql: avoid calling sql_encode_table_opts() during trigger creation Nikita Pettik
2018-12-10 14:17   ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-11 18:29     ` n.pettik
2018-12-09 21:30 ` [tarantool-patches] [PATCH 2/6] sql: don't update SQL string during renaming Nikita Pettik
2018-12-10 14:16   ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-11 18:29     ` n.pettik
2018-12-12 12:36       ` Vladislav Shpilevoy
2018-12-13 12:42         ` n.pettik
2018-12-09 21:30 ` [tarantool-patches] [PATCH 3/6] test: fix sqltester methods to drop all tables/views Nikita Pettik
2018-12-10 14:16   ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-11 18:29     ` n.pettik
2018-12-09 21:30 ` [tarantool-patches] [PATCH 4/6] sql: don't add string of 'CREATE TABLE...' to space opts Nikita Pettik
2018-12-10 14:17   ` Vladislav Shpilevoy [this message]
2018-12-11 18:29     ` [tarantool-patches] " n.pettik
2018-12-09 21:30 ` [tarantool-patches] [PATCH 5/6] sql: don't add string of 'CREATE INDEX ...' to index opts Nikita Pettik
2018-12-10 14:18   ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-11 18:29     ` n.pettik
2018-12-09 21:30 ` [tarantool-patches] [PATCH 6/6] Remove SQL string from " Nikita Pettik
2018-12-25 13:45 ` [tarantool-patches] Re: [PATCH 0/6] Remove string of SQL statement from opts Kirill Yukhin

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=6965549c-a919-0460-2603-b4765a32e338@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH 4/6] sql: don'\''t add string of '\''CREATE TABLE...'\'' to space opts' \
    /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