[tarantool-patches] Re: [PATCH v2 1/1] sql: disallow returning many rows from subselect

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

> 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