From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 7A69725B5B for ; Mon, 18 Jun 2018 07:06:21 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id AXi9OqWqYfrr for ; Mon, 18 Jun 2018 07:06:21 -0400 (EDT) Received: from smtp35.i.mail.ru (smtp35.i.mail.ru [94.100.177.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 30E0C25B54 for ; Mon, 18 Jun 2018 07:06:21 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH] sql: remove redundant goto from VDBE prologue References: <20180612182607.53450-1-korablev@tarantool.org> <24a1a33e-02cc-ccf4-7ff9-a155f5e18781@tarantool.org> <28322F56-E52D-42C7-AE52-188D6F50AF63@tarantool.org> From: Vladislav Shpilevoy Message-ID: <5b137003-b3ea-918d-4528-a1c30712abd5@tarantool.org> Date: Mon, 18 Jun 2018 14:06:18 +0300 MIME-Version: 1.0 In-Reply-To: <28322F56-E52D-42C7-AE52-188D6F50AF63@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: "n.pettik" , tarantool-patches@freelists.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.