[tarantool-patches] Re: [PATCH] sql: remove tests on ORDER BY/LIMIT + DELETE
n.pettik
korablev at tarantool.org
Fri Feb 8 17:22:32 MSK 2019
> On 7 Feb 2019, at 21:54, n.pettik <korablev at tarantool.org> wrote:
>
> LGTM.
Oh, my apologies, I forget that I've already removed this define and tests
connected with it:
https://github.com/tarantool/tarantool/commit/43ed060f9539ed44a232ca81ad890b1099872cd9
I’m really sorry, we should skip this patch.
>> On 7 Feb 2019, at 20:45, i.koptelov <ivan.koptelov at tarantool.org <mailto:ivan.koptelov at tarantool.org>> wrote:
>>
>>>>
>>>> On 4 Feb 2019, at 22:11, Ivan Koptelov <ivan.koptelov at tarantool.org <mailto:ivan.koptelov at tarantool.org>> wrote:
>>>>>> On 4 Feb 2019, at 18:34, Ivan Koptelov <ivan.koptelov at tarantool.org <mailto:ivan.koptelov at tarantool.org>>
>>>>>> wrote:
>>>>>>
>>>>>> Removes obsolete tests on ORDER BY/LIMIT + DELETE. This
>>>>>> functionality was not supported for some time, but a few tests
>>>>>> were still existing.
>>>>>>
>>>>>> Closes #2172
>>>>>> ---
>>>>>> Branch
>>>>>> https://github.com/tarantool/tarantool/compare/sudobobo/gh-2172-rm-order-by-and-limit-from-delete <https://github.com/tarantool/tarantool/compare/sudobobo/gh-2172-rm-order-by-and-limit-from-delete>
>>>>>>
>>>>>> Issue
>>>>>> https://github.com/tarantool/tarantool/issues/2172
>>>>>>
>>>>>>
>>>>>> test/sql-tap/e_delete.test.lua | 188 -----------------------------------------
>>>>>> 1 file changed, 188 deletions(-)
>>>>>>
>>>>>> diff --git a/test/sql-tap/e_delete.test.lua b/test/sql-tap/e_delete.test.lua
>>>>>> index 84a4e0a22..0a81eb3bd 100755
>>>>>> --- a/test/sql-tap/e_delete.test.lua
>>>>>> +++ b/test/sql-tap/e_delete.test.lua
>>>>>> @@ -305,194 +305,6 @@ if (0 > 0) then
>>>>>> DELETE FROM t8 NOT INDEXED WHERE a=5;
>>>>>> END;]], {"NOT INDEXED"}},
>>>>>> })
>>>>>> - -- EVIDENCE-OF: R-64942-06615 The LIMIT and ORDER BY clauses (described
>>>>>> - -- below) are unsupported for DELETE statements within triggers.
>>>>>> - —
>>>>>>
>>>>> If I’m not mistaken, tests starting from e_delete-2.0 are disabled
>>>>> (under if 0 > 0). So you can delete even more tests.
>>>>>
>>>> I thought we could enable this tests later?
>>>
>>> Could you please examine those tests. If they are valid, simply uncomment them.
>>> If not so - remove. Do it in a separate patch/commit pls.
>> They are not valid and test functionality we don’t have. I would send a separate small patch
>> with removing these tests.
>>
>>>>> Also, check SQLITE_ENABLE_UPDATE_DELETE_LIMIT define:
>>>>> I guess it is related to this patch. Seems that it is turned off, so I
>>>>> assume we can delete it alongside with code under it.
>>>>>
>>>> Yes, it turned off currently, but it may be turned on. This option also enables
>>>> UPDATE + LIMIT/ORDER BY statements and all code under 'define' appplies
>>>> to this functionality as well, so firstly I decided not to delete it.
>>>
>>> I really doubt that some day we will do it. LIMIT is non-ansi extension.
>>> For example Oracle doesn’t support it at all. I guess this is a reason
>>> why we got rid of delete+limit. Lets remove it as well.
>> Done.
>>
>> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
>> index 0bcf41594..646e95ebe 100644
>> --- a/src/box/sql/parse.y
>> +++ b/src/box/sql/parse.y
>> @@ -727,7 +727,6 @@ where_opt(A) ::= WHERE expr(X). {A = X.pExpr;}
>>
>> ////////////////////////// The UPDATE command ////////////////////////////////
>> //
>> -%ifndef SQLITE_ENABLE_UPDATE_DELETE_LIMIT
>> cmd ::= with(C) UPDATE orconf(R) fullname(X) indexed_opt(I) SET setlist(Y)
>> where_opt(W). {
>> sqlite3WithPush(pParse, C, 1);
>> @@ -738,7 +737,6 @@ cmd ::= with(C) UPDATE orconf(R) fullname(X) indexed_opt(I) SET setlist(Y)
>> pParse->initiateTTrans = true;
>> sqlite3Update(pParse,X,Y,W,R);
>> }
>> -%endif
>>
>> %type setlist {ExprList*}
>> %destructor setlist {sql_expr_list_delete(pParse->db, $$);}
>> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
>> index 6462467bc..4c6baaac6 100644
>> --- a/src/box/sql/resolve.c
>> +++ b/src/box/sql/resolve.c
>> @@ -579,26 +579,6 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
>> }
>> #endif
>> switch (pExpr->op) {
>> -
>> -#if defined(SQLITE_ENABLE_UPDATE_DELETE_LIMIT)
>> - /* The special operator TK_ROW means use the rowid for the first
>> - * column in the FROM clause. This is used by the LIMIT and ORDER BY
>> - * clause processing on UPDATE and DELETE statements.
>> - */
>> - case TK_ROW:{
>> - SrcList *pSrcList = pNC->pSrcList;
>> - struct SrcList_item *pItem;
>> - assert(pSrcList && pSrcList->nSrc == 1);
>> - pItem = pSrcList->a;
>> - pExpr->op = TK_COLUMN;
>> - pExpr->pTab = pItem->pTab;
>> - pExpr->iTable = pItem->iCursor;
>> - pExpr->iColumn = -1;
>> - pExpr->affinity = AFFINITY_INTEGER;
>> - break;
>> - }
>> -#endif /* defined(SQLITE_ENABLE_UPDATE_DELETE_LIMIT) */
>> -
>> /* A lone identifier is the name of a column.
>> */
>> case TK_ID:{
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20190208/cb96459e/attachment.html>
More information about the Tarantool-patches
mailing list