Tarantool development patches archive
 help / color / mirror / Atom feed
From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH] sql: rework VIEW internals
Date: Fri, 8 Jun 2018 16:17:04 +0300	[thread overview]
Message-ID: <374C812C-1387-42D1-B1B1-EE8FEDAA70A4@tarantool.org> (raw)
In-Reply-To: <c3bf8b1d-ca2f-0d32-5f2b-d685d0182d93@tarantool.org>


> On 7 Jun 2018, at 23:06, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Thanks for the fixes! See 1 comment below and a commit with my
> fixes on the branch right after your. Most likely, my fixes are
> not passing tests, so please, finish them. I fixed Select * leak on
> exception during trigger_new_xc, and error message. + some style
> fixes.

Thanks for fixes, I have squashed them and tests seem to pass
(Except for 1 run: box/sql.test.lua once failed on timeout, but I’m not sure
that it is my fault: locally on my machine it works fine both as single run and within suite).
The only change I made - added fiber.sleep(0.1) in tests after
box.error.injection.set("ERRINJ_WAL_DELAY", false)
Without it test turns out to be flaky.

>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index 28e4d7a4d..53bb53ab8 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -2009,230 +2007,99 @@ sqlite3EndTable(Parse * pParse,	/* Parse context */
>> int
>> -sql_view_column_names(struct Parse *parse, struct Table *table)
>> +sql_view_assign_cursors(struct Parse *parse, const char *view_stmt)
>> {
>> +	assert(view_stmt != NULL);
>> +	struct sqlite3 *db = parse->db;
>> +	struct Select *select = sql_view_compile(db, view_stmt);
>> 	if (select == NULL)
>> 		return -1;
>> -	int n = parse->nTab;
>> 	sqlite3SrcListAssignCursors(parse, select->pSrc);
> 
> Where do you free the struct Select?

Nowhere, shame on me..

+++ b/src/box/sql/build.c
@@ -2090,6 +2090,7 @@ sql_view_assign_cursors(struct Parse *parse, const char *view_stmt)
        if (select == NULL)
                return -1;
        sqlite3SrcListAssignCursors(parse, select->pSrc);
+       sql_select_delete(db, select);

  reply	other threads:[~2018-06-08 13:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-04 16:26 [tarantool-patches] " Nikita Pettik
2018-06-05 11:30 ` [tarantool-patches] " Vladislav Shpilevoy
2018-06-06 14:25   ` n.pettik
2018-06-07 10:40     ` Vladislav Shpilevoy
2018-06-07 18:25       ` n.pettik
2018-06-07 20:06         ` Vladislav Shpilevoy
2018-06-08 13:17           ` n.pettik [this message]
2018-06-08 20:05             ` Vladislav Shpilevoy
2018-06-19 13:04               ` Kirill Yukhin
2018-06-08  4:08         ` Konstantin Osipov

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=374C812C-1387-42D1-B1B1-EE8FEDAA70A4@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH] sql: rework VIEW internals' \
    /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