[tarantool-patches] [PATCH v1 1/1] sql: assertion fault on select after analyze

Kirill Shcherbatov kshcherbatov at tarantool.org
Fri Jul 6 16:30:41 MSK 2018


Binary search in whereKeyStats function that use routine
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.

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
variable may be used twice:

          [74, 'Column', 4, 1, 11, '', '00', 'r[11]=']
          [75, 'Column', 4, 2, 12, '', '00', 'r[12]=']
|->       [76, 'Column', 4, 3, 13, '', '00', 'r[13]=']
          [77, 'Column', 4, 4, 14, '', '00', 'r[14]=']
!rewrite  [78, 'Column', 4, 0, 13, '', '00', 'r[13]=ID']
          [79, 'MakeRecord', 13, 1, 6, '', '00',
           'r[6]=mkrec(r[13])']

Thus all assumptions about data below should be invalid and
some artifacts could exhibit.

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 = 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]]
---

Expected:
---
['T1', 'I1', '101 101 101 101 1', '0 0 0 0 44', '0 0 0 0 44',
 ['x', 'y', 'z', 'a', 44]]
---

Resolves #2847.
---

https://github.com/tarantool/tarantool/compare/kshch/gh-2847-assertion-fault-on-analyze
https://github.com/tarantool/tarantool/issues/2847

 src/box/sql/analyze.c          |   6 +-
 test/sql-tap/analyze4.test.lua |   2 +-
 test/sql-tap/analyze9.test.lua |  16 ++---
 test/sql/analyze.result        | 152 +++++++++++++++++++++++++++++++++++++++++
 test/sql/analyze.test.lua      |  28 ++++++++
 5 files changed, 193 insertions(+), 11 deletions(-)
 create mode 100644 test/sql/analyze.result
 create mode 100644 test/sql/analyze.test.lua

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 = sqlite3PrimaryKeyIndex(pIdx->pTable);
 		int j, k, regKeyStat;
 		int nPkColumn = (int)index_column_count(pPk);
-		regKeyStat = sqlite3GetTempRange(pParse, nPkColumn);
+		/* Allocate memory for array. */
+		pParse->nMem = MAX(pParse->nMem, regPrev + nColTest + nPkColumn);
+		regKeyStat = regPrev + nColTest;
+
 		for (j = 0; j < nPkColumn; j++) {
 			k = pPk->aiColumn[j];
 			assert(k >= 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);
 
 		assert(regChng == (regStat4 + 1));
 		sqlite3VdbeAddOp4(v, OP_Function0, 1, regStat4, regTemp,
diff --git a/test/sql-tap/analyze4.test.lua b/test/sql-tap/analyze4.test.lua
index f71d394..c2cc190 100755
--- a/test/sql-tap/analyze4.test.lua
+++ b/test/sql-tap/analyze4.test.lua
@@ -115,7 +115,7 @@ test:do_execsql_test(
     ]]
     , {
         -- <analyze4-1.3>
-        "T1","128 1", "T1A", "128 1", "T1B", "128 128", "T1BCD", "128 128 4 2", "T1CBD", "128 4 4 2", "T1CDB", "128 4 2 1"
+        "T1","128 1", "T1A", "128 1", "T1B", "128 128", "T1BCD", "128 128 4 2", "T1CBD", "128 4 4 2", "T1CDB", "128 4 2 2"
         -- </analyze4-1.3>
     })
 
diff --git a/test/sql-tap/analyze9.test.lua b/test/sql-tap/analyze9.test.lua
index 28a6c20..1dbfe5d 100755
--- a/test/sql-tap/analyze9.test.lua
+++ b/test/sql-tap/analyze9.test.lua
@@ -287,13 +287,13 @@ test:do_execsql_test(
             FROM "_sql_stat4" WHERE "idx" = 'I1' ORDER BY "sample" LIMIT 16;
     ]], {
         -- <4.3>
-        "10 10 1","0 0 8","0 0 8","0 0 0","10 10 1","10 10 10","1 1 10","1 1 1","10 10 1","20 20 28",
-        "2 2 28","2 2 2","10 10 1","30 30 33","3 3 33","3 3 3","10 10 1","40 40 45","4 4 45","4 4 4",
-        "10 10 1","50 50 51","5 5 51","5 5 5","10 10 1","60 60 67","6 6 67","6 6 6","10 10 1","70 70 74",
-        "7 7 74","7 7 7","10 10 1","80 80 82","8 8 82","8 8 8","10 10 1","90 90 96","9 9 96","9 9 9",
-        "10 10 1","100 100 101","10 10 101","10 10 10","10 10 1","110 110 116","11 11 116","11 11 11",
-        "10 10 1","120 120 122","12 12 122","12 12 12","10 10 1","130 130 135","13 13 135","13 13 13",
-        "10 10 1","140 140 148","14 14 148","14 14 14","10 10 1","150 150 151","15 15 151","15 15 15"
+        "10 10 10","0 0 0","0 0 0","0 0 0","10 10 10","10 10 10","1 1 1","1 1 1","10 10 10","20 20 20",
+        "2 2 2","2 2 2","10 10 10","30 30 30","3 3 3","3 3 3","10 10 10","40 40 40","4 4 4","4 4 4",
+        "10 10 10","50 50 50","5 5 5","5 5 5","10 10 10","60 60 60","6 6 6","6 6 6","10 10 10","70 70 70",
+        "7 7 7","7 7 7","10 10 10","80 80 80","8 8 8","8 8 8","10 10 10","90 90 90","9 9 9","9 9 9",
+        "10 10 10","100 100 100","10 10 10","10 10 10","10 10 10","110 110 110","11 11 11","11 11 11",
+        "10 10 10","120 120 120","12 12 12","12 12 12","10 10 10","130 130 130","13 13 13","13 13 13",
+        "10 10 10","140 140 140","14 14 14","14 14 14","10 10 10","150 150 150","15 15 15","15 15 15"
         -- </4.3>
     })
 
@@ -304,7 +304,7 @@ test:do_execsql_test(
         FROM "_sql_stat4" WHERE "idx" = 'I1' ORDER BY "sample" DESC LIMIT 2;
     ]], {
         -- <4.4>
-        "2 1 1","295 296 296","120 122 296","201 4 h","5 3 1","290 290 291","119 119 291","200 1 b"
+        "2 1 1","295 296 296","120 122 125","201 4 h","5 3 1","290 290 291","119 119 120","200 1 b"
         -- </4.4>
     })
 
diff --git a/test/sql/analyze.result b/test/sql/analyze.result
new file mode 100644
index 0000000..cbe7971
--- /dev/null
+++ b/test/sql/analyze.result
@@ -0,0 +1,152 @@
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+--
+-- gh-2847: assertion fault on select after analyze
+--
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+
+box.cfg{};
+---
+...
+box.sql.execute("DROP TABLE IF EXISTS t1;");
+---
+...
+box.sql.execute("CREATE TABLE t1(id INTEGER PRIMARY KEY AUTOINCREMENT, a, b, c, d, e);");
+---
+...
+box.sql.execute("CREATE INDEX i1 ON t1(a, b, c, d);");
+---
+...
+box.sql.execute("CREATE INDEX i2 ON t1(e);");
+---
+...
+for i = 0, 100 do
+    box.sql.execute(string.format("INSERT INTO t1 VALUES(null, 'x', 'y', 'z', %s, %s);", i, math.floor(i / 2)))
+end;
+---
+...
+for i = 0, 20 do
+    box.sql.execute("INSERT INTO t1 VALUES(null, 'x', 'y', 'z', 101, "..i..");")
+end;
+---
+...
+for i = 102, 200 do
+    box.sql.execute(string.format("INSERT INTO t1 VALUES(null, 'x', 'y', 'z', %s, %s);", i, math.floor(i / 2)))
+end;
+---
+...
+box.sql.execute("ANALYZE;")
+box.sql.execute("SELECT * FROM \"_sql_stat1\"");
+---
+...
+box.sql.execute("SELECT * FROM \"_sql_stat4\"");
+---
+- - ['T1', 'I1', '221 221 221 1', '0 0 0 10', '0 0 0 10', !!binary lKF4oXmhego=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 24', '0 0 0 24', !!binary lKF4oXmhehg=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 25', '0 0 0 25', !!binary lKF4oXmhehk=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 32', '0 0 0 32', !!binary lKF4oXmheiA=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 35', '0 0 0 35', !!binary lKF4oXmheiM=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 44', '0 0 0 44', !!binary lKF4oXmheiw=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 49', '0 0 0 49', !!binary lKF4oXmhejE=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 56', '0 0 0 56', !!binary lKF4oXmhejg=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 62', '0 0 0 62', !!binary lKF4oXmhej4=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 66', '0 0 0 66', !!binary lKF4oXmhekI=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 74', '0 0 0 74', !!binary lKF4oXmheko=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 80', '0 0 0 80', !!binary lKF4oXmhelA=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 99', '0 0 0 99', !!binary lKF4oXmhemM=]
+  - ['T1', 'I1', '221 221 221 21', '0 0 0 101', '0 0 0 101', !!binary lKF4oXmhemU=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 124', '0 0 0 104', !!binary lKF4oXmhemg=]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 149', '0 0 0 129', !!binary lKF4oXmhesyB]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 157', '0 0 0 137', !!binary lKF4oXmhesyJ]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 174', '0 0 0 154', !!binary lKF4oXmhesya]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 176', '0 0 0 156', !!binary lKF4oXmhesyc]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 185', '0 0 0 165', !!binary lKF4oXmhesyl]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 192', '0 0 0 172', !!binary lKF4oXmhesys]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 199', '0 0 0 179', !!binary lKF4oXmhesyz]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 210', '0 0 0 190', !!binary lKF4oXmhesy+]
+  - ['T1', 'I1', '221 221 221 1', '0 0 0 219', '0 0 0 199', !!binary lKF4oXmheszH]
+  - ['T1', 'I2', '3', '0', '0', !!binary kQA=]
+  - ['T1', 'I2', '3', '3', '1', !!binary kQE=]
+  - ['T1', 'I2', '3', '6', '2', !!binary kQI=]
+  - ['T1', 'I2', '3', '12', '4', !!binary kQQ=]
+  - ['T1', 'I2', '3', '15', '5', !!binary kQU=]
+  - ['T1', 'I2', '3', '18', '6', !!binary kQY=]
+  - ['T1', 'I2', '3', '21', '7', !!binary kQc=]
+  - ['T1', 'I2', '3', '24', '8', !!binary kQg=]
+  - ['T1', 'I2', '3', '27', '9', !!binary kQk=]
+  - ['T1', 'I2', '3', '30', '10', !!binary kQo=]
+  - ['T1', 'I2', '3', '33', '11', !!binary kQs=]
+  - ['T1', 'I2', '3', '36', '12', !!binary kQw=]
+  - ['T1', 'I2', '3', '39', '13', !!binary kQ0=]
+  - ['T1', 'I2', '3', '42', '14', !!binary kQ4=]
+  - ['T1', 'I2', '3', '45', '15', !!binary kQ8=]
+  - ['T1', 'I2', '3', '48', '16', !!binary kRA=]
+  - ['T1', 'I2', '3', '57', '19', !!binary kRM=]
+  - ['T1', 'I2', '3', '60', '20', !!binary kRQ=]
+  - ['T1', 'I2', '2', '73', '26', !!binary kRo=]
+  - ['T1', 'I2', '2', '99', '39', !!binary kSc=]
+  - ['T1', 'I2', '2', '124', '52', !!binary kTQ=]
+  - ['T1', 'I2', '2', '148', '64', !!binary kUA=]
+  - ['T1', 'I2', '2', '174', '77', !!binary kU0=]
+  - ['T1', 'I2', '2', '198', '89', !!binary kVk=]
+  - ['T1', 'T1', '1', '2', '2', !!binary kQM=]
+  - ['T1', 'T1', '1', '6', '6', !!binary kQc=]
+  - ['T1', 'T1', '1', '12', '12', !!binary kQ0=]
+  - ['T1', 'T1', '1', '24', '24', !!binary kRk=]
+  - ['T1', 'T1', '1', '31', '31', !!binary kSA=]
+  - ['T1', 'T1', '1', '33', '33', !!binary kSI=]
+  - ['T1', 'T1', '1', '34', '34', !!binary kSM=]
+  - ['T1', 'T1', '1', '43', '43', !!binary kSw=]
+  - ['T1', 'T1', '1', '49', '49', !!binary kTI=]
+  - ['T1', 'T1', '1', '66', '66', !!binary kUM=]
+  - ['T1', 'T1', '1', '74', '74', !!binary kUs=]
+  - ['T1', 'T1', '1', '94', '94', !!binary kV8=]
+  - ['T1', 'T1', '1', '99', '99', !!binary kWQ=]
+  - ['T1', 'T1', '1', '101', '101', !!binary kWY=]
+  - ['T1', 'T1', '1', '108', '108', !!binary kW0=]
+  - ['T1', 'T1', '1', '112', '112', !!binary kXE=]
+  - ['T1', 'T1', '1', '124', '124', !!binary kX0=]
+  - ['T1', 'T1', '1', '127', '127', !!binary kcyA]
+  - ['T1', 'T1', '1', '149', '149', !!binary kcyW]
+  - ['T1', 'T1', '1', '152', '152', !!binary kcyZ]
+  - ['T1', 'T1', '1', '153', '153', !!binary kcya]
+  - ['T1', 'T1', '1', '174', '174', !!binary kcyv]
+  - ['T1', 'T1', '1', '187', '187', !!binary kcy8]
+  - ['T1', 'T1', '1', '199', '199', !!binary kczI]
+...
+box.sql.execute("SELECT * FROM t1 WHERE a='x' AND b='y' AND c='z' AND d=101");
+---
+- - [102, 'x', 'y', 'z', 101, 0]
+  - [103, 'x', 'y', 'z', 101, 1]
+  - [104, 'x', 'y', 'z', 101, 2]
+  - [105, 'x', 'y', 'z', 101, 3]
+  - [106, 'x', 'y', 'z', 101, 4]
+  - [107, 'x', 'y', 'z', 101, 5]
+  - [108, 'x', 'y', 'z', 101, 6]
+  - [109, 'x', 'y', 'z', 101, 7]
+  - [110, 'x', 'y', 'z', 101, 8]
+  - [111, 'x', 'y', 'z', 101, 9]
+  - [112, 'x', 'y', 'z', 101, 10]
+  - [113, 'x', 'y', 'z', 101, 11]
+  - [114, 'x', 'y', 'z', 101, 12]
+  - [115, 'x', 'y', 'z', 101, 13]
+  - [116, 'x', 'y', 'z', 101, 14]
+  - [117, 'x', 'y', 'z', 101, 15]
+  - [118, 'x', 'y', 'z', 101, 16]
+  - [119, 'x', 'y', 'z', 101, 17]
+  - [120, 'x', 'y', 'z', 101, 18]
+  - [121, 'x', 'y', 'z', 101, 19]
+  - [122, 'x', 'y', 'z', 101, 20]
+...
+box.sql.execute("DROP TABLE t1");
+---
+...
diff --git a/test/sql/analyze.test.lua b/test/sql/analyze.test.lua
new file mode 100644
index 0000000..efc1a68
--- /dev/null
+++ b/test/sql/analyze.test.lua
@@ -0,0 +1,28 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+test_run:cmd("setopt delimiter ';'")
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+--
+-- gh-2847: assertion fault on select after analyze
+--
+
+box.cfg{};
+box.sql.execute("DROP TABLE IF EXISTS t1;");
+box.sql.execute("CREATE TABLE t1(id INTEGER PRIMARY KEY AUTOINCREMENT, a, b, c, d, e);");
+box.sql.execute("CREATE INDEX i1 ON t1(a, b, c, d);");
+box.sql.execute("CREATE INDEX i2 ON t1(e);");
+for i = 0, 100 do
+    box.sql.execute(string.format("INSERT INTO t1 VALUES(null, 'x', 'y', 'z', %s, %s);", i, math.floor(i / 2)))
+end;
+for i = 0, 20 do
+    box.sql.execute("INSERT INTO t1 VALUES(null, 'x', 'y', 'z', 101, "..i..");")
+end;
+for i = 102, 200 do
+    box.sql.execute(string.format("INSERT INTO t1 VALUES(null, 'x', 'y', 'z', %s, %s);", i, math.floor(i / 2)))
+end;
+box.sql.execute("ANALYZE;")
+box.sql.execute("SELECT * FROM \"_sql_stat1\"");
+box.sql.execute("SELECT * FROM \"_sql_stat4\"");
+box.sql.execute("SELECT * FROM t1 WHERE a='x' AND b='y' AND c='z' AND d=101");
+box.sql.execute("DROP TABLE t1");
-- 
2.7.4





More information about the Tarantool-patches mailing list