[tarantool-patches] Re: [PATCH] sql: remove redundant goto from VDBE prologue

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Mon Jun 18 14:06:18 MSK 2018


>>
>> 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.




More information about the Tarantool-patches mailing list