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 CDD81292B4 for ; Fri, 1 Jun 2018 04:56:41 -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 Jq5huiambYNC for ; Fri, 1 Jun 2018 04:56:41 -0400 (EDT) Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41]) (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 2BA51292AC for ; Fri, 1 Jun 2018 04:56:41 -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] sql: fix decode analyze sample From: "n.pettik" In-Reply-To: <20180518134733.4490-1-avkhatskevich@tarantool.org> Date: Fri, 1 Jun 2018 11:56:35 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <85302581-2D9C-448F-B593-364C246D6175@tarantool.org> References: <20180518134733.4490-1-avkhatskevich@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: Alex Khatskevich > On 18 May 2018, at 16:47, AKhatskevich = wrote: >=20 > branch: kh/gh-2860-skip-scan >=20 > Analyze samples are encoded with msgpack and should be decoded as it = is > a msgpack. >=20 > Closes #2860 > =E2=80=94 Put here link to the branch on GitHub, and link to the issue. > src/box/sql/sqliteInt.h | 2 +- > src/box/sql/vdbemem.c | 35 +++++++------------------ > src/box/sql/where.c | 4 +-- > test/sql-tap/whereG.test.lua | 61 = +++++++++++++++++++++++++++++++++++++++++++- > 4 files changed, 72 insertions(+), 30 deletions(-) >=20 > diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h > index 2800be583..a5b33fe71 100644 > --- a/src/box/sql/sqliteInt.h > +++ b/src/box/sql/sqliteInt.h > @@ -4020,7 +4020,7 @@ int sqlite3Stat4ProbeSetValue(Parse *, Index *, = UnpackedRecord **, Expr *, int, > int, int *); > int sqlite3Stat4ValueFromExpr(Parse *, Expr *, u8, sqlite3_value **); > void sqlite3Stat4ProbeFree(UnpackedRecord *); > -int sqlite3Stat4Column(sqlite3 *, const void *, int, int, = sqlite3_value **); > +int sqlite3Stat4Column(sqlite3 *, const void *, int, sqlite3_value = **); > char sqlite3IndexColumnAffinity(sqlite3 *, Index *, int); >=20 > /* > diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c > index f91d7119c..70e03adda 100644 > --- a/src/box/sql/vdbemem.c > +++ b/src/box/sql/vdbemem.c > @@ -1589,7 +1589,7 @@ sqlite3Stat4ValueFromExpr(Parse * pParse, = /* Parse context */ > } >=20 > /* > - * Extract the iCol-th column from the nRec-byte record in pRec. = Write > + * Extract the iCol-th column from the record in pRec. Write > * the column value into *ppVal. If *ppVal is initially NULL then a = new > * sqlite3_value object is allocated. > * > @@ -1599,44 +1599,27 @@ sqlite3Stat4ValueFromExpr(Parse * pParse, = /* Parse context */ > int > sqlite3Stat4Column(sqlite3 * db, /* Database handle */ > const void *pRec, /* Pointer to buffer containing = record */ > - int nRec, /* Size of buffer pRec in bytes */ > int iCol, /* Column to extract */ > sqlite3_value ** ppVal /* OUT: Extracted value = */ > ) I see that provided changes significantly refactor this function. Would you mind to fix code style for whole function to make it look like function from server? > { > - u32 t; /* a column type code */ > - int nHdr; /* Size of the header in the record */ > - int iHdr; /* Next unread header byte */ > - int iField; /* Next unread data byte */ > - int szField =3D 0; /* Size of the current data field */ > int i; /* Column index */ > - u8 *a =3D (u8 *) pRec; /* Typecast byte array */ > + const char *a =3D (const char *) pRec; /* Typecast byte array = */ > Mem *pMem =3D *ppVal; /* Write result into this Mem object */ > - > - assert(iCol > 0); > - iHdr =3D getVarint32(a, nHdr); > - if (nHdr > nRec || iHdr >=3D nHdr) > + assert(iCol >=3D 0); > + assert(mp_typeof(a[0]) =3D=3D MP_ARRAY); > + int n_col =3D mp_decode_array(&a); > + if (n_col <=3D iCol) > return SQLITE_CORRUPT_BKPT; Lest just return 0 in case of success, and -1 otherwise. > - iField =3D nHdr; > - for (i =3D 0; i <=3D iCol; i++) { > - iHdr +=3D getVarint32(&a[iHdr], t); > - testcase(iHdr =3D=3D nHdr); > - testcase(iHdr =3D=3D nHdr + 1); > - if (iHdr > nHdr) > - return SQLITE_CORRUPT_BKPT; > - szField =3D sqlite3VdbeSerialTypeLen(t); > - iField +=3D szField; > + for (i =3D 0; i < iCol; i++) { > + mp_next(&a); > } We don=E2=80=99t surround one-line if/for with brackets. > - testcase(iField =3D=3D nRec); > - testcase(iField =3D=3D nRec + 1); > - if (iField > nRec) > - return SQLITE_CORRUPT_BKPT; > if (pMem =3D=3D 0) { > pMem =3D *ppVal =3D sqlite3ValueNew(db); > if (pMem =3D=3D 0) > return SQLITE_NOMEM_BKPT; > } > - sqlite3VdbeSerialGet(&a[iField - szField], t, pMem); > + sqlite3VdbeMsgpackGet((const unsigned char *) a, pMem); > return SQLITE_OK; > } >=20 > diff --git a/src/box/sql/where.c b/src/box/sql/where.c > index cde38152a..a4a1aee98 100644 > --- a/src/box/sql/where.c > +++ b/src/box/sql/where.c > @@ -1183,8 +1183,8 @@ whereRangeSkipScanEst(Parse * pParse, = /* Parsing & code generating context */ > int i; > int nDiff; > for (i =3D 0; rc =3D=3D SQLITE_OK && i < p->nSample; = i++) { > - rc =3D sqlite3Stat4Column(db, p->aSample[i].p, > - p->aSample[i].n, nEq, = &pVal); > + rc =3D sqlite3Stat4Column(db, p->aSample[i].p, = nEq, > + &pVal); > if (rc =3D=3D SQLITE_OK && p1) { > int res =3D sqlite3MemCompare(p1, pVal, = pColl); > if (res >=3D 0) > diff --git a/test/sql-tap/whereG.test.lua = b/test/sql-tap/whereG.test.lua > index 1842f2ae8..68b8ce488 100755 > --- a/test/sql-tap/whereG.test.lua > +++ b/test/sql-tap/whereG.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test =3D require("sqltester") > -test:plan(20) > +test:plan(22) >=20 > --!./tcltestrunner.lua > -- 2013-09-05 > @@ -388,5 +388,64 @@ test:do_execsql_test( > -- > }) >=20 > +-- gh-2860 skip-scan plan > +test:execsql([[ > + CREATE TABLE people(name TEXT PRIMARY KEY, role TEXT NOT NULL, > + height INT NOT NULL, CHECK(role IN ('student','teacher'))); > + CREATE INDEX people_idx1 ON people(role, height); > + INSERT INTO people VALUES('Alice','student',156); > + INSERT INTO people VALUES('Bob','student',161); > + INSERT INTO people VALUES('Cindy','student',155); > + INSERT INTO people VALUES('David','student',181); > + INSERT INTO people VALUES('Emily','teacher',158); > + INSERT INTO people VALUES('Fred','student',163); > + INSERT INTO people VALUES('Ginny','student',169); > + INSERT INTO people VALUES('Harold','student',172); > + INSERT INTO people VALUES('Imma','student',179); > + INSERT INTO people VALUES('Jack','student',181); > + INSERT INTO people VALUES('Karen','student',163); > + INSERT INTO people VALUES('Logan','student',177); > + INSERT INTO people VALUES('Megan','teacher',159); > + INSERT INTO people VALUES('Nathan','student',163); > + INSERT INTO people VALUES('Olivia','student',161); > + INSERT INTO people VALUES('Patrick','teacher',180); > + INSERT INTO people VALUES('Quiana','student',182); > + INSERT INTO people VALUES('Robert','student',159); > + INSERT INTO people VALUES('Sally','student',166); > + INSERT INTO people VALUES('Tom','student',171); > + INSERT INTO people VALUES('Ursula','student',170); > + INSERT INTO people VALUES('Vance','student',179); > + INSERT INTO people VALUES('Willma','student',175); > + INSERT INTO people VALUES('Xavier','teacher',185); > + INSERT INTO people VALUES('Yvonne','student',149); > + INSERT INTO people VALUES('Zach','student',170); > + INSERT INTO people VALUES('Angie','student',166); > + INSERT INTO people VALUES('Brad','student',176); > + INSERT INTO people VALUES('Claire','student',168); > + INSERT INTO people VALUES('Donald','student',162); > + INSERT INTO people VALUES('Elaine','student',177); > + INSERT INTO people VALUES('Frazier','student',159); > + INSERT INTO people VALUES('Grace','student',179); > + INSERT INTO people VALUES('Horace','student',166); > + INSERT INTO people VALUES('Ingrad','student',155); > + INSERT INTO people VALUES('Jacob','student',179); > + INSERT INTO people VALUES('John', 'student', 154); > + ANALYZE; > +]]) > + > +test:do_execsql_test( > + "7.1", > + [[ > + SELECT name FROM people WHERE height>=3D180; > + ]], > + {"David","Jack","Quiana","Patrick","Xavier"}) > + > +test:do_execsql_test( > + "7.2", > + [[ > + EXPLAIN QUERY PLAN SELECT name FROM people WHERE height>=3D180;= > + ]], Lets execute the same select before ANALYZE to make sure that plan has = changed. > + {0,0,0,"SEARCH TABLE PEOPLE USING COVERING INDEX PEOPLE_IDX1" .. > + " (ANY(ROLE) AND HEIGHT>?)"}) >=20 > test:finish_test() > --=20 > 2.14.1 >=20 >=20