[tarantool-patches] [PATCH v2] sql: fix decode analyze sample

[tarantool-patches] [PATCH v2] sql: fix decode analyze sample

[AKhatskevich]

Analyze samples are encoded with msgpack and should be decoded as a
msgpack.

sqlite3Stat4Column sqlite-style function converted to sql_stat4_column
Tarantool-styled.

Closes #2860
---
Changes:
 * sqlite3Stat4Column Tarantool style

Branch: https://github.com/tarantool/tarantool/tree/kh/gh-2860-skip-scan
Issue: https://github.com/tarantool/tarantool/issues/2860

 src/box/sql/sqliteInt.h      | 20 ++++++++++++-
 src/box/sql/vdbemem.c        | 66 +++++++++++++-----------------------------
 src/box/sql/where.c          |  3 +-
 test/sql-tap/whereG.test.lua | 68 +++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 107 insertions(+), 50 deletions(-)

diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 2800be583..b64075959 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4020,7 +4020,25 @@ 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 **);
+
+/*
+ * 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.
+ *
+ * If *ppVal is initially NULL then the caller is responsible for
+ * ensuring that the value written into *ppVal is eventually
+ * freed.
+ *
+ * @param db Database handle.
+ * @param pRec Pointer to buffer containing record.
+ * @param iCol Column to extract.
+ * @param[out] ppVal Extracted value.
+ *
+ * @retval Error code or SQLITE_OK.
+ */
+int
+sql_stat4_column(struct 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..821d4db74 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -1588,55 +1588,29 @@ sqlite3Stat4ValueFromExpr(Parse * pParse,	/* Parse context */
 	return stat4ValueFromExpr(pParse, pExpr, affinity, 0, ppVal);
 }
 
-/*
- * Extract the iCol-th column from the nRec-byte record in pRec.  Write
- * the column value into *ppVal.  If *ppVal is initially NULL then a new
- * sqlite3_value object is allocated.
- *
- * If *ppVal is initially NULL then the caller is responsible for
- * ensuring that the value written into *ppVal is eventually freed.
- */
 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 */
-    )
+sql_stat4_column(struct sqlite3 *db, const void *record, int col_num,
+		 sqlite3_value **res)
 {
-	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 */
-	Mem *pMem = *ppVal;	/* Write result into this Mem object */
-
-	assert(iCol > 0);
-	iHdr = getVarint32(a, nHdr);
-	if (nHdr > nRec || iHdr >= nHdr)
-		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;
-	}
-	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;
+	assert(col_num >= 0);
+	/* Write result into this Mem object */
+	Mem *mem = *res;
+	/* Typecast byte array */
+	const char *a = (const char *) record;
+	assert(mp_typeof(a[0]) == MP_ARRAY);
+	int col_cnt = mp_decode_array(&a);
+	assert(col_cnt > col_num);
+	for (int i = 0; i < col_num; i++)
+		mp_next(&a);
+	if (mem == 0) {
+		mem = *res = sqlite3ValueNew(db);
+		if (mem == 0) {
+			diag_set(OutOfMemory, sizeof(Mem), "sqlite3ValeuNew",
+				 "mem");
+			return -1;
+		}
 	}
-	sqlite3VdbeSerialGet(&a[iField - szField], t, pMem);
+	sqlite3VdbeMsgpackGet((const unsigned char *) record, mem);
 	return SQLITE_OK;
 }
 
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index cde38152a..4a76ed2ec 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -1183,8 +1183,7 @@ 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 = sql_stat4_column(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..13cef16c8 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(23)
 
 --!./tcltestrunner.lua
 -- 2013-09-05
@@ -388,5 +388,71 @@ 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);
+]])
+
+test:do_execsql_test(
+    "7.1",
+    [[
+        SELECT name FROM people WHERE height>=180 order by name;
+    ]],
+    {"David","Jack","Patrick","Quiana","Xavier"})
+
+test:do_execsql_test(
+    "7.2",
+    [[
+        EXPLAIN QUERY PLAN SELECT name FROM people WHERE height>=180;
+    ]],
+    {0,0,0,"SCAN TABLE PEOPLE"})
+
+test:do_execsql_test(
+    "7.3",
+    [[
+        ANALYZE;
+        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

[tarantool-patches] Re: [PATCH v2] sql: fix decode analyze sample

[n.pettik]

Except for comments below, pls rebase on fresh 2.0.
Your branch seems to be way behind current master.

> int sqlite3Stat4ValueFromExpr(Parse *, Expr *, u8, sqlite3_value **);
> void sqlite3Stat4ProbeFree(UnpackedRecord *);
> -int sqlite3Stat4Column(sqlite3 *, const void *, int, int, sqlite3_value **);
> +
> +/*

Start comment with /**

> + * Extract the iCol-th column from the record in pRec.  Write
> + * the column value into *ppVal.  If *ppVal is initially NULL

Lets move from Hungarian notation to Tarantool-style naming.
Moreover, you didn’t specify args names in func prototype.

> + * then a new sqlite3_value object is allocated.
> + *
> + * If *ppVal is initially NULL then the caller is responsible for
> + * ensuring that the value written into *ppVal is eventually
> + * freed.
> + *
> + * @param db Database handle.
> + * @param pRec Pointer to buffer containing record.
> + * @param iCol Column to extract.
> + * @param[out] ppVal Extracted value.
> + *
> + * @retval Error code or SQLITE_OK.

Or SQLITE_OK on success (or 0 after fixes).

> + */
> +int
> +sql_stat4_column(struct 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..821d4db74 100644
> --- a/src/box/sql/vdbemem.c
> +++ b/src/box/sql/vdbemem.c
> @@ -1588,55 +1588,29 @@ sqlite3Stat4ValueFromExpr(Parse * pParse,	/* Parse context */
> 	return stat4ValueFromExpr(pParse, pExpr, affinity, 0, ppVal);
> }
> 
> -/*
> - * Extract the iCol-th column from the nRec-byte record in pRec.  Write
> - * the column value into *ppVal.  If *ppVal is initially NULL then a new
> - * sqlite3_value object is allocated.
> - *
> - * If *ppVal is initially NULL then the caller is responsible for
> - * ensuring that the value written into *ppVal is eventually freed.
> - */
> 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 */
> -    )
> +sql_stat4_column(struct sqlite3 *db, const void *record, int col_num,
> +		 sqlite3_value **res)
> {
> -	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 */
> -	Mem *pMem = *ppVal;	/* Write result into this Mem object */
> -
> -	assert(iCol > 0);
> -	iHdr = getVarint32(a, nHdr);
> -	if (nHdr > nRec || iHdr >= nHdr)
> -		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;
> -	}
> -	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;
> +	assert(col_num >= 0);
> +	/* Write result into this Mem object */

Put dots at the end of sentences.

> +	Mem *mem = *res;

Lets use struct prefix.

> +	/* Typecast byte array */
> +	const char *a = (const char *) record;

Lets pass already const char*. AFAIK all routine connected with MsgPack
accepts const char *, but not void *.

> +	assert(mp_typeof(a[0]) == MP_ARRAY);
> +	int col_cnt = mp_decode_array(&a);
> +	assert(col_cnt > col_num);
> +	for (int i = 0; i < col_num; i++)
> +		mp_next(&a);
> +	if (mem == 0) {

== NULL

> +		mem = *res = sqlite3ValueNew(db);

Don’t use double assignments.

> +		if (mem == 0) {

== NULL

> +			diag_set(OutOfMemory, sizeof(Mem), "sqlite3ValeuNew”,

sqlite3ValueNew

> +				 "mem");
> +			return -1;
> +		}
> 	}
> -	sqlite3VdbeSerialGet(&a[iField - szField], t, pMem);
> +	sqlite3VdbeMsgpackGet((const unsigned char *) record, mem);
> 	return SQLITE_OK;

Just return 0 (in fact, they are the same thing).

[tarantool-patches] Re: [PATCH v2] sql: fix decode analyze sample

[Alex Khatskevich]

> Except for comments below, pls rebase on fresh 2.0.
> Your branch seems to be way behind current master
> Start comment with /**
>> + * Extract the iCol-th column from the record in pRec.  Write
>> + * the column value into *ppVal.  If *ppVal is initially NULL
> Lets move from Hungarian notation to Tarantool-style naming.
> Moreover, you didn’t specify args names in func prototype.

done

> Or SQLITE_OK on success (or 0 after fixes).
ok, sorry
> Put dots at the end of sentences.
> Lets use struct prefix.
> Lets pass already const char*. AFAIK all routine connected with MsgPack
> accepts const char *, but not void *.
>
> == NULL
> Don’t use double assignments.
> == NULL
> sqlite3ValueNew
> Just return 0 (in fact, they are the same thing).
done

Here is a full diff

commit c4e932a313cc7cd01db611856cf277df9e73305d
Author: AKhatskevich <avkhatskevich@tarantool.org>
Date:   Fri May 18 13:56:53 2018 +0300

     sql: fix decode analyze sample

     Analyze samples are encoded with msgpack and should be decoded as a
     msgpack.

     sqlite3Stat4Column sqlite-style function converted to sql_stat4_column
     Tarantool-styled.

     Closes #2860

diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 878daa8df..837371aa7 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4383,7 +4383,26 @@ 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 
**);
+
+/**
+ * Extract the col_num-th column from the record.  Write
+ * the column value into *res.  If *res is initially NULL
+ * then a new sqlite3_value object is allocated.
+ *
+ * If *res is initially NULL then the caller is responsible for
+ * ensuring that the value written into *res is eventually
+ * freed.
+ *
+ * @param db Database handle.
+ * @param record Pointer to buffer containing record.
+ * @param col_num Column to extract.
+ * @param[out] res Extracted value.
+ *
+ * @retval -1 on error or 0.
+ */
+int
+sql_stat4_column(struct sqlite3 *db, const char *record, int col_num,
+         sqlite3_value **res);
  char sqlite3IndexColumnAffinity(sqlite3 *, Index *, int);

  /*
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index f408b7701..017109359 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -1588,56 +1588,31 @@ sqlite3Stat4ValueFromExpr(Parse * pParse,    /* 
Parse context */
      return stat4ValueFromExpr(pParse, pExpr, affinity, 0, ppVal);
  }

-/*
- * Extract the iCol-th column from the nRec-byte record in pRec.  Write
- * the column value into *ppVal.  If *ppVal is initially NULL then a new
- * sqlite3_value object is allocated.
- *
- * If *ppVal is initially NULL then the caller is responsible for
- * ensuring that the value written into *ppVal is eventually freed.
- */
  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 */
-    )
+sql_stat4_column(struct sqlite3 *db, const char *record, int col_num,
+         sqlite3_value **res)
  {
-    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 */
-    Mem *pMem = *ppVal;    /* Write result into this Mem object */
-
-    assert(iCol > 0);
-    iHdr = getVarint32(a, nHdr);
-    if (nHdr > nRec || iHdr >= nHdr)
-        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;
-    }
-    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;
+    assert(col_num >= 0);
+    /* Write result into this Mem object. */
+    struct Mem *mem = *res;
+    /* Typecast byte array. */
+    const char *a = record;
+    assert(mp_typeof(a[0]) == MP_ARRAY);
+    int col_cnt = mp_decode_array(&a);
+    assert(col_cnt > col_num);
+    for (int i = 0; i < col_num; i++)
+        mp_next(&a);
+    if (mem == NULL) {
+        mem = sqlite3ValueNew(db);
+        *res = mem;
+        if (mem == NULL) {
+            diag_set(OutOfMemory, sizeof(Mem), "sqlite3ValueNew",
+                 "mem");
+            return -1;
+        }
      }
-    sqlite3VdbeSerialGet(&a[iField - szField], t, pMem);
-    return SQLITE_OK;
+    sqlite3VdbeMsgpackGet((const unsigned char *) a, mem);
+    return 0;
  }

  /*
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index f017384b5..4bbd39a73 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -1245,8 +1245,8 @@ whereRangeSkipScanEst(Parse * pParse,        /* 
Parsing & code generating context */
          struct index_sample *samples = index->def->opts.stat->samples;
          uint32_t sample_count = index->def->opts.stat->sample_count;
          for (i = 0; rc == SQLITE_OK && i < (int) sample_count; i++) {
-            rc = sqlite3Stat4Column(db, samples[i].sample_key,
-                        samples[i].key_size, nEq, &pVal);
+            rc = sql_stat4_column(db, samples[i].sample_key, 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..13cef16c8 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(23)

  --!./tcltestrunner.lua
  -- 2013-09-05
@@ -388,5 +388,71 @@ 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);
+]])
+
+test:do_execsql_test(
+    "7.1",
+    [[
+        SELECT name FROM people WHERE height>=180 order by name;
+    ]],
+    {"David","Jack","Patrick","Quiana","Xavier"})
+
+test:do_execsql_test(
+    "7.2",
+    [[
+        EXPLAIN QUERY PLAN SELECT name FROM people WHERE height>=180;
+    ]],
+    {0,0,0,"SCAN TABLE PEOPLE"})
+
+test:do_execsql_test(
+    "7.3",
+    [[
+        ANALYZE;
+        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()

[tarantool-patches] Re: [PATCH v2] sql: fix decode analyze sample

[n.pettik]

As far as I understand, there is only one call of sql_stat4_column().
You may make this function static and move to where.c,
if you want so.

> index f408b7701..017109359 100644
> --- a/src/box/sql/vdbemem.c
> +++ b/src/box/sql/vdbemem.c
> @@ -1588,56 +1588,31 @@ sqlite3Stat4ValueFromExpr(Parse * pParse,    /* Parse context */
>      return stat4ValueFromExpr(pParse, pExpr, affinity, 0, ppVal);
>  }
> 
> +    assert(col_num >= 0);
> +    /* Write result into this Mem object. */
> +    struct Mem *mem = *res;
> +    /* Typecast byte array. */

I guess, it is obsolete comment.

> +    const char *a = record;
> +    assert(mp_typeof(a[0]) == MP_ARRAY);
> +    int col_cnt = mp_decode_array(&a);

mp_decode_array returns uint32_t, so lets use this type.

> +    assert(col_cnt > col_num);
> +    for (int i = 0; i < col_num; i++)
> +        mp_next(&a);
> +    if (mem == NULL) {
> +        mem = sqlite3ValueNew(db);
> +        *res = mem;
> +        if (mem == NULL) {
> +            diag_set(OutOfMemory, sizeof(Mem), "sqlite3ValueNew”,

sizeof(struct Mem) or *mem

The rest seems OK to me.

[tarantool-patches] Re: [PATCH v2] sql: fix decode analyze sample

[Alex Khatskevich]

> As far as I understand, there is only one call of sql_stat4_column().
> You may make this function static and move to where.c,
> if you want so.
Ok.
Let it be here by now.
> +    /* Typecast byte array. */
> I guess, it is obsolete comment.
Deleted
>> +    int col_cnt = mp_decode_array(&a);
> mp_decode_array returns uint32_t, so lets use this type.
Fixed
>> +        if (mem == NULL) {
>> +            diag_set(OutOfMemory, sizeof(Mem), "sqlite3ValueNew”,
> sizeof(struct Mem) or *mem
Fixed
> The rest seems OK to me.

[tarantool-patches] Re: [PATCH v2] sql: fix decode analyze sample

[n.pettik]

Now LGTM.

[tarantool-patches] Re: [PATCH v2] sql: fix decode analyze sample

[Kirill Yukhin]

On 29 июн 18:05, n.pettik wrote:
> Now LGTM.
Checked into 2.0 branch.

--
Regards, Kirill Yukhin