* [tarantool-patches] [PATCH] sql: fix decode analyze sample
@ 2018-05-18 13:47 AKhatskevich
2018-06-01 8:56 ` [tarantool-patches] " n.pettik
0 siblings, 1 reply; 3+ messages in thread
From: AKhatskevich @ 2018-05-18 13:47 UTC (permalink / raw)
To: tarantool-patches
branch: kh/gh-2860-skip-scan
Analyze samples are encoded with msgpack and should be decoded as it is
a msgpack.
Closes #2860
---
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(-)
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);
/*
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 */
}
/*
- * 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 */
)
{
- 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 = 0; /* Size of the current data field */
int i; /* Column index */
- u8 *a = (u8 *) pRec; /* Typecast byte array */
+ const char *a = (const char *) pRec; /* Typecast byte array */
Mem *pMem = *ppVal; /* Write result into this Mem object */
-
- assert(iCol > 0);
- iHdr = getVarint32(a, nHdr);
- if (nHdr > nRec || iHdr >= nHdr)
+ assert(iCol >= 0);
+ assert(mp_typeof(a[0]) == MP_ARRAY);
+ int n_col = mp_decode_array(&a);
+ if (n_col <= iCol)
return SQLITE_CORRUPT_BKPT;
- iField = nHdr;
- for (i = 0; i <= iCol; i++) {
- iHdr += getVarint32(&a[iHdr], t);
- testcase(iHdr == nHdr);
- testcase(iHdr == nHdr + 1);
- if (iHdr > nHdr)
- return SQLITE_CORRUPT_BKPT;
- szField = sqlite3VdbeSerialTypeLen(t);
- iField += szField;
+ for (i = 0; i < iCol; i++) {
+ mp_next(&a);
}
- testcase(iField == nRec);
- testcase(iField == nRec + 1);
- if (iField > nRec)
- return SQLITE_CORRUPT_BKPT;
if (pMem == 0) {
pMem = *ppVal = sqlite3ValueNew(db);
if (pMem == 0)
return SQLITE_NOMEM_BKPT;
}
- sqlite3VdbeSerialGet(&a[iField - szField], t, pMem);
+ sqlite3VdbeMsgpackGet((const unsigned char *) a, pMem);
return SQLITE_OK;
}
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 = 0; rc == SQLITE_OK && i < p->nSample; i++) {
- rc = sqlite3Stat4Column(db, p->aSample[i].p,
- p->aSample[i].n, nEq, &pVal);
+ rc = sqlite3Stat4Column(db, p->aSample[i].p, nEq,
+ &pVal);
if (rc == SQLITE_OK && p1) {
int res = sqlite3MemCompare(p1, pVal, pColl);
if (res >= 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 = require("sqltester")
-test:plan(20)
+test:plan(22)
--!./tcltestrunner.lua
-- 2013-09-05
@@ -388,5 +388,64 @@ test:do_execsql_test(
-- </7.3>
})
+-- 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>=180;
+ ]],
+ {"David","Jack","Quiana","Patrick","Xavier"})
+
+test:do_execsql_test(
+ "7.2",
+ [[
+ EXPLAIN QUERY PLAN SELECT name FROM people WHERE height>=180;
+ ]],
+ {0,0,0,"SEARCH TABLE PEOPLE USING COVERING INDEX PEOPLE_IDX1" ..
+ " (ANY(ROLE) AND HEIGHT>?)"})
test:finish_test()
--
2.14.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: fix decode analyze sample
2018-05-18 13:47 [tarantool-patches] [PATCH] sql: fix decode analyze sample AKhatskevich
@ 2018-06-01 8:56 ` n.pettik
[not found] ` <79583087-87c1-a36d-92c2-9daaf8be7772@tarantool.org>
0 siblings, 1 reply; 3+ messages in thread
From: n.pettik @ 2018-06-01 8:56 UTC (permalink / raw)
To: tarantool-patches; +Cc: Alex Khatskevich
> On 18 May 2018, at 16:47, AKhatskevich <avkhatskevich@tarantool.org> wrote:
>
> branch: kh/gh-2860-skip-scan
>
> Analyze samples are encoded with msgpack and should be decoded as it is
> a msgpack.
>
> Closes #2860
> —
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(-)
>
> 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);
>
> /*
> 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 */
> }
>
> /*
> - * 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 = 0; /* Size of the current data field */
> int i; /* Column index */
> - u8 *a = (u8 *) pRec; /* Typecast byte array */
> + const char *a = (const char *) pRec; /* Typecast byte array */
> Mem *pMem = *ppVal; /* Write result into this Mem object */
> -
> - assert(iCol > 0);
> - iHdr = getVarint32(a, nHdr);
> - if (nHdr > nRec || iHdr >= nHdr)
> + assert(iCol >= 0);
> + assert(mp_typeof(a[0]) == MP_ARRAY);
> + int n_col = mp_decode_array(&a);
> + if (n_col <= iCol)
> return SQLITE_CORRUPT_BKPT;
Lest just return 0 in case of success, and -1 otherwise.
> - iField = nHdr;
> - for (i = 0; i <= iCol; i++) {
> - iHdr += getVarint32(&a[iHdr], t);
> - testcase(iHdr == nHdr);
> - testcase(iHdr == nHdr + 1);
> - if (iHdr > nHdr)
> - return SQLITE_CORRUPT_BKPT;
> - szField = sqlite3VdbeSerialTypeLen(t);
> - iField += szField;
> + for (i = 0; i < iCol; i++) {
> + mp_next(&a);
> }
We don’t surround one-line if/for with brackets.
> - testcase(iField == nRec);
> - testcase(iField == nRec + 1);
> - if (iField > nRec)
> - return SQLITE_CORRUPT_BKPT;
> if (pMem == 0) {
> pMem = *ppVal = sqlite3ValueNew(db);
> if (pMem == 0)
> return SQLITE_NOMEM_BKPT;
> }
> - sqlite3VdbeSerialGet(&a[iField - szField], t, pMem);
> + sqlite3VdbeMsgpackGet((const unsigned char *) a, pMem);
> return SQLITE_OK;
> }
>
> 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 = 0; rc == SQLITE_OK && i < p->nSample; i++) {
> - rc = sqlite3Stat4Column(db, p->aSample[i].p,
> - p->aSample[i].n, nEq, &pVal);
> + rc = sqlite3Stat4Column(db, p->aSample[i].p, nEq,
> + &pVal);
> if (rc == SQLITE_OK && p1) {
> int res = sqlite3MemCompare(p1, pVal, pColl);
> if (res >= 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 = require("sqltester")
> -test:plan(20)
> +test:plan(22)
>
> --!./tcltestrunner.lua
> -- 2013-09-05
> @@ -388,5 +388,64 @@ test:do_execsql_test(
> -- </7.3>
> })
>
> +-- 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>=180;
> + ]],
> + {"David","Jack","Quiana","Patrick","Xavier"})
> +
> +test:do_execsql_test(
> + "7.2",
> + [[
> + EXPLAIN QUERY PLAN SELECT name FROM people WHERE height>=180;
> + ]],
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>?)"})
>
> test:finish_test()
> --
> 2.14.1
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-06-01 12:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-18 13:47 [tarantool-patches] [PATCH] sql: fix decode analyze sample AKhatskevich
2018-06-01 8:56 ` [tarantool-patches] " n.pettik
[not found] ` <79583087-87c1-a36d-92c2-9daaf8be7772@tarantool.org>
2018-06-01 12:08 ` n.pettik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox