[tarantool-patches] Re: [PATCH v1 1/1] sql: assertion fault on select after analyze
- From: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
- To: tarantool-patches@xxxxxxxxxxxxx, Nikita Pettik <korablev@xxxxxxxxxxxxx>
- Date: Fri, 13 Jul 2018 20:07:45 +0300
Hi! Tnx for review.
Typo: uses
Typo: But.
fixed.
Cound —> count?
could. fixed.
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.
https://github.com/tarantool/tarantool/issues/3533
Don’t create separate file for one issue - there already exist
analyze[1-F] tests. Put this to one of them.
moved to analyze1
============================================
Binary search in whereKeyStats function that uses routine
sqlite3VdbeRecordCompareMsgpack returned invalid result,
because it made wrong assumption about the number of
coinciding index columns based on _sql_stat4.
But 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 could 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.
---
src/box/sql/analyze.c | 6 +++--
test/sql-tap/analyze1.test.lua | 59 +++++++++++++++++++++++++++++++++++++++++-
test/sql-tap/analyze4.test.lua | 2 +-
test/sql-tap/analyze9.test.lua | 16 ++++++------
4 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index ea872f4..ea93bfd 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -997,7 +997,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);
@@ -1007,7 +1010,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/analyze1.test.lua b/test/sql-tap/analyze1.test.lua
index 308f6b9..dab7255 100755
--- a/test/sql-tap/analyze1.test.lua
+++ b/test/sql-tap/analyze1.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(34)
+test:plan(38)
--!./tcltestrunner.lua
-- 2005 July 22
@@ -489,6 +489,63 @@ test:do_execsql_test(
"T4"
-- </analyze-5.5>
})
+
+test:do_test(
+ "analyze-6.1.1",
+ function()
+ test:execsql("DROP TABLE IF EXISTS t1 ")
+ test:execsql("CREATE TABLE t1(id INTEGER PRIMARY KEY AUTOINCREMENT, a,
b, c, d, e);")
+ test:execsql("CREATE INDEX i1 ON t1(a, b, c, d);")
+ test:execsql("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;
+
+ test:execsql("ANALYZE;")
+ return test:execsql("SELECT COUNT(* )FROM t1 WHERE a='x' AND b='y' AND
c='z' AND d=101;;")
+ end, {
+ -- <analyze-6.1.1>
+ 21
+ -- </analyze-6.1.1>
+})
+
+test:do_execsql_test(
+ "analyze-6.1.2",
+ [[
+ SELECT * FROM "_sql_stat1" where "tbl"='T1' and "idx"='I1' LIMIT 1;
+ ]], {
+ -- <analyze-6.1.2>
+ "T1", "I1", "221 221 221 221 2"
+ -- </analyze-6.1.2>
+})
+
+test:do_execsql_test(
+ "analyze-6.1.3",
+ [[
+ SELECT "tbl", "idx", "neq", "nlt", "ndlt" FROM "_sql_stat4" where
"tbl"='T1' and "idx"='I1' ORDER BY "nlt" LIMIT 1;
+ ]], {
+ -- <analyze-6.1.3>
+ "T1", "I1", "221 221 221 1", "0 0 0 10", "0 0 0 10"
+ -- </analyze-6.1.3>
+})
+
+test:do_execsql_test(
+ "analyze-6.1.4",
+ [[
+ SELECT "tbl", "idx", "neq", "nlt", "ndlt" FROM "_sql_stat4" where
"tbl"='T1' and "idx"='I1' ORDER BY "nlt" DESC LIMIT 1;
+ ]], {
+ -- <analyze-6.1.4>
+ "T1", "I1", "221 221 221 1", "0 0 0 99", "0 0 0 99"
+ -- </analyze-6.1.4>
+})
+
-- # This test corrupts the database file so it must be the last test
-- # in the series.
-- #
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>
})
--
2.7.4
Other related posts:
