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 0FC9F22C0E for ; Mon, 2 Jul 2018 10:14:31 -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 kIJCBis0FHCG for ; Mon, 2 Jul 2018 10:14:30 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 B7BAC22BC3 for ; Mon, 2 Jul 2018 10:14:29 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect References: <7815ec2cb5cfb0d837478cff88fe4ba95503e8d7.1530113937.git.kshcherbatov@tarantool.org> <80d9fc88-3030-764b-55f2-86d6e5e33f03@tarantool.org> <411BF67B-D49B-468C-80EF-AF31D520F0DF@tarantool.org> <7ba69010-2e21-9e4f-d923-4061ddecf4cf@tarantool.org> From: Kirill Shcherbatov Message-ID: <1ea436b1-2471-2b8a-8223-c12095308994@tarantool.org> Date: Mon, 2 Jul 2018 17:14:26 +0300 MIME-Version: 1.0 In-Reply-To: <7ba69010-2e21-9e4f-d923-4061ddecf4cf@tarantool.org> Content-Type: text/plain; charset=utf-8 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: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy > And I have pushed more minor fixes on the branch. Please, > squash. Ok, tnx. > 1. How can it be != NULL? If user has manually wrote LIMIT expression, like this SELECT b FROM t1 WHERE a = (SELECT a FROM t1 WHERE b=6 LIMIT 1); or this SELECT b FROM t1 WHERE a = (SELECT a FROM t1 WHERE b=6 LIMIT (SELECT b-1 FROM t1 WHERE a =1)); > 2. ExprSerProperty does not check for pSel->pLimit == NULL, so it crashes on > OOM. - ExprSetProperty(pSel->pLimit, EP_System); + if (pSel->pLimit != NULL) { + ExprSetProperty(pSel->pLimit, + EP_System); + } (If pLimit is NULL sqlite3Select returns with -1 at the beginning). > 3. I do not see where do you set an appropriate error code (p5). > 4. Same. @@ -2145,6 +2145,7 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, 0, 0, error, P4_STATIC); + sqlite3VdbeChangeP5(v, ER_SQL_EXECUTE); sqlite3VdbeResolveLabel(v, no_err); sqlite3ReleaseTempReg(pParse, r1); @@ -5452,6 +5453,7 @@ vdbe_code_raise_on_multiple_rows(struct Parse *parser, int limit_reg, int end_ma sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, ON_CONFLICT_ACTION_FAIL, 0, error, P4_STATIC); + sqlite3VdbeChangeP5(v, SQL_TARANTOOL_ERROR); > 5. Why do you have two runtime checks for the same auto limit? There are in totally different branches, with no intersection on runtime checks. The prev. one make return few lines bellow vdbe_code_raise_on_multiple_rows. > 6. Why not 0x20000? No reason. -#define SF_SingleRow 0x40000 +#define SF_SingleRow 0x20000