Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: "n.pettik" <korablev@tarantool.org>, tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH] sql: remove redundant goto from VDBE prologue
Date: Mon, 18 Jun 2018 14:06:18 +0300	[thread overview]
Message-ID: <5b137003-b3ea-918d-4528-a1c30712abd5@tarantool.org> (raw)
In-Reply-To: <28322F56-E52D-42C7-AE52-188D6F50AF63@tarantool.org>


>>
>> 3. As far as I see, P2 in OP_Init is 1 already when we are here. It is
>> not? See allocVdbe function. P2 == 1 by default, and here it can be changed to
>> goto to ttrans.
> 
> In fact, it is changed by sqlite3VdbeJumpHere(v, 0);
> Thus, we have to again set its value to 1, in case of omitting jump.

It is done in the same function few lines above. How about
to do not do this jump + not jump? I have slightly refactored the
code to do not this jump. Please, see the separate commit on
the branch.

(I did not check the tests).

> 
>>
>>>   	}
>>>     	/* Get the VDBE program ready for execution
>>
>> 4. Can we test the new VDBE plan using EXPLAIN? I am wondering why all
>> plan changes are not tested using EXPLAIN.
> 
> Well, it it is quite complicated to test EXPLAIN command since any change to query
> planner or code generator at all would result in rewriting such tests. Hence, once we
> decided to avoid using tests with EXPLAIN until we get stable code generation.

I think, that we should start test plans. At least, here it is possible to call
EXPLAIN, test first result line only and ignore others. Then this test will not
fail on any plan change.

  reply	other threads:[~2018-06-18 11:06 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-12 18:26 [tarantool-patches] " Nikita Pettik
2018-06-17 20:08 ` [tarantool-patches] " Vladislav Shpilevoy
2018-06-18 10:46   ` n.pettik
2018-06-18 11:06     ` Vladislav Shpilevoy [this message]
2018-06-18 17:45       ` n.pettik
2018-06-18 18:57         ` Vladislav Shpilevoy
2018-06-18 20:55           ` n.pettik
2018-06-18 21:03             ` Vladislav Shpilevoy
2018-06-19  8:21               ` 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=5b137003-b3ea-918d-4528-a1c30712abd5@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH] sql: remove redundant goto from VDBE prologue' \
    /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