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 B65DA27488 for ; Fri, 13 Jul 2018 11:43:54 -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 A76Fg7A7Eo1V for ; Fri, 13 Jul 2018 11:43:54 -0400 (EDT) Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (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 0276B2747A for ; Fri, 13 Jul 2018 11:43:53 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: assertion fault on select after analyze From: "n.pettik" In-Reply-To: <84f7314ef474adcd50bd68738f2df301dbc08662.1530883770.git.kshcherbatov@tarantool.org> Date: Fri, 13 Jul 2018 18:43:50 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <84f7314ef474adcd50bd68738f2df301dbc08662.1530883770.git.kshcherbatov@tarantool.org> 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: Kirill Shcherbatov > On 6 Jul 2018, at 16:30, Kirill Shcherbatov = wrote: >=20 > Binary search in whereKeyStats function that use routine Typo: uses > sqlite3VdbeRecordCompareMsgpack returned invalid result, > because it made wrong assumption about the number of > coinciding index columns based on _sql_stat4. > Bat all collected index statistics were wrong. Typo: But. >=20 > The analyzeOneTable routine used to call sqlite3GetTempRange > and sqlite3ReleaseTempRange functions inside of cycle intensively > changes Parser->nMem value. Thanks this pattern routine > sqlite3ReleaseTempRange could push nMem variable onto aTempReg > queue. As Parser->nMem cound be bigger next time, this Cound =E2=80=94> count? > variable may be used twice: >=20 > [74, 'Column', 4, 1, 11, '', '00', 'r[11]=3D'] > [75, 'Column', 4, 2, 12, '', '00', 'r[12]=3D'] > |-> [76, 'Column', 4, 3, 13, '', '00', 'r[13]=3D'] > [77, 'Column', 4, 4, 14, '', '00', 'r[14]=3D'] > !rewrite [78, 'Column', 4, 0, 13, '', '00', 'r[13]=3DID'] > [79, 'MakeRecord', 13, 1, 6, '', '00', > 'r[6]=3Dmkrec(r[13])'] >=20 > Thus all assumptions about data below should be invalid and > some artifacts could exhibit. >=20 > Example: > \set language sql > CREATE TABLE t1(id INTEGER PRIMARY KEY AUTOINCREMENT, a, > b, c, f, d, e); > CREATE INDEX i1 ON t1(a, b, c, f, d); > \set language lua > for i =3D 0, 100 do > box.sql.execute(string.format("INSERT INTO t1 VALUES(null, > 'x', 'y', 'z', 'a', %s, %s);", i, > math.floor(i / 2))) > end > \set language sql > SELECT * FROM "_sql_stat4"; > --- > ['T1', 'I1', '101 101 1 1 1', '0 0 44 44 44', '0 0 44 44 44', > ['x', 'y', 'z', 'a', 44]] > --- >=20 > Expected: > --- > ['T1', 'I1', '101 101 101 101 1', '0 0 0 0 44', '0 0 0 0 44', > ['x', 'y', 'z', 'a', 44]] > --- >=20 > Resolves #2847. > =E2=80=94 Thanks for investigating and fixing this bug. But problem seems to be = deeper (as you can notice). Am OK with this particular fix, but we must unify usage of nMem and range of registers (i.e. whether use everywhere raw nMem and get rid of TempReg function or vice versa). Please, open an issue describing misusage of sqlite3GetTempReg() and = parser->nMem. > diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c > index 5f73f02..e8eec76 100644 > --- a/src/box/sql/analyze.c > +++ b/src/box/sql/analyze.c > @@ -1024,7 +1024,10 @@ analyzeOneTable(Parse * pParse, /* = Parser context */ > Index *pPk =3D sqlite3PrimaryKeyIndex(pIdx->pTable); > int j, k, regKeyStat; > int nPkColumn =3D (int)index_column_count(pPk); > - regKeyStat =3D sqlite3GetTempRange(pParse, nPkColumn); > + /* Allocate memory for array. */ > + pParse->nMem =3D MAX(pParse->nMem, regPrev + nColTest + = nPkColumn); > + regKeyStat =3D regPrev + nColTest; > + > for (j =3D 0; j < nPkColumn; j++) { > k =3D pPk->aiColumn[j]; > assert(k >=3D 0 && k < = (int)pTab->def->field_count); > @@ -1034,7 +1037,6 @@ analyzeOneTable(Parse * pParse, /* Parser = context */ > } > sqlite3VdbeAddOp3(v, OP_MakeRecord, regKeyStat, > nPkColumn, regKey); > - sqlite3ReleaseTempRange(pParse, regKeyStat, nPkColumn); >=20 > assert(regChng =3D=3D (regStat4 + 1)); > diff --git a/test/sql/analyze.test.lua b/test/sql/analyze.test.lua Don=E2=80=99t create separate file for one issue - there already exist analyze[1-F] tests. Put this to one of them.