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 364D426902 for ; Tue, 5 Mar 2019 12:54:52 -0500 (EST) 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 MWDcbsZxSTSb for ; Tue, 5 Mar 2019 12:54:52 -0500 (EST) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 B935526455 for ; Tue, 5 Mar 2019 12:54:51 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH] sql: store statistics in statN as an array of integers From: "n.pettik" In-Reply-To: Date: Tue, 5 Mar 2019 20:54:49 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <443EE5E9-93B1-4F19-9C71-C28F97A754CB@tarantool.org> References: 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: Roman Khabibov I pushed some refactoring at the top of your branch. Check it and if it is OK, merge commits. I also attach it to the letter: diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua index ab46142ba..fde91b7d9 100644 --- a/src/box/lua/upgrade.lua +++ b/src/box/lua/upgrade.lua @@ -615,10 +615,10 @@ local function upgrade_to_2_1_0() end =20 = --------------------------------------------------------------------------= ------ --- Tarantool 2.1.1 +-- Tarantool 2.1.2 = --------------------------------------------------------------------------= ------ =20 -local function upgrade_sql_stat_to_2_1_1() +local function upgrade_to_2_1_2() local _sql_stat1 =3D box.space[box.schema.SQL_STAT1_ID] local _sql_stat4 =3D box.space[box.schema.SQL_STAT4_ID] =20 @@ -626,7 +626,6 @@ local function upgrade_sql_stat_to_2_1_1() format[3].type =3D 'array' _sql_stat1:format(format) =20 - format =3D _sql_stat4:format() format[3].type =3D 'array' format[4].type =3D 'array' @@ -634,10 +633,6 @@ local function upgrade_sql_stat_to_2_1_1() _sql_stat4:format(format) end =20 -local function upgrade_to_2_1_1() - upgrade_sql_stat_to_2_1_1() -end - local function get_version() local version =3D box.space._schema:get{'version'} if version =3D=3D nil then @@ -666,7 +661,7 @@ local function upgrade(options) {version =3D mkversion(1, 10, 0), func =3D upgrade_to_1_10_0, = auto =3D true}, {version =3D mkversion(1, 10, 2), func =3D upgrade_to_1_10_2, = auto =3D true}, {version =3D mkversion(2, 1, 0), func =3D upgrade_to_2_1_0, = auto =3D true}, - {version =3D mkversion(2, 1, 1), func =3D upgrade_to_2_1_1, = auto =3D true} + {version =3D mkversion(2, 1, 2), func =3D upgrade_to_2_1_2, = auto =3D true} } =20 for _, handler in ipairs(handlers) do diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c index cfc987693..94940ef28 100644 --- a/src/box/sql/analyze.c +++ b/src/box/sql/analyze.c @@ -645,7 +645,7 @@ statGet(sql_context * context, int argc, sql_value = ** argv) /* Return the value to store in the "stat" column of the = _sql_stat1 * table for this index. * - * The value is a msg pack array composed of a list of = integers describing + * The value is an array composed of a list of integers = describing * the index. The first integer in the list is the total = number of * entries in the index. There is one additional integer = in the list * for each indexed column. This additional integer is = an estimate of @@ -663,35 +663,30 @@ statGet(sql_context * context, int argc, sql_value = ** argv) * * I =3D (K+D-1)/D */ - int i; - - size_t size =3D mp_sizeof_array(p->nKeyCol + 1); + uint32_t size =3D mp_sizeof_array(p->nKeyCol + 1); size +=3D mp_sizeof_uint(p->nRow); - for (i =3D 0; i < p->nKeyCol; ++i) { + for (int i =3D 0; i < p->nKeyCol; ++i) { uint64_t dist_count =3D p->current.anDLt[i] + 1; uint64_t val =3D (p->nRow + dist_count - 1) / = dist_count; size +=3D mp_sizeof_uint(val); } - char *z_ret =3D sqlMallocZero(size); - if (z_ret =3D=3D 0) { + char *mp_stat1 =3D malloc(size); + if (mp_stat1 =3D=3D NULL) { sql_result_error_nomem(context); return; } - char *z =3D z_ret; - z =3D mp_encode_array(z, p->nKeyCol + 1); - z =3D mp_encode_uint(z, p->nRow); - for (i =3D 0; i < p->nKeyCol; ++i) { + char *data =3D mp_stat1; + data =3D mp_encode_array(data, p->nKeyCol + 1); + data =3D mp_encode_uint(data, p->nRow); + for (int i =3D 0; i < p->nKeyCol; ++i) { uint64_t dist_count =3D p->current.anDLt[i] + 1; uint64_t val =3D (p->nRow + dist_count - 1) / = dist_count; - z =3D mp_encode_uint(z, val); - assert(p->current.anEq[i]); + data =3D mp_encode_uint(data, val); + assert(p->current.anEq[i] > 0); } - const char *b =3D z_ret; - assert(mp_check(&b, z) =3D=3D 0); - (void) b; - assert(z_ret !=3D z); + assert(data =3D=3D size + mp_stat1); =20 - sql_result_blob(context, z_ret, size, sql_free); + sql_result_blob(context, mp_stat1, size, free); context->pOut->flags|=3D MEM_Subtype; context->pOut->subtype =3D SQL_SUBTYPE_MSGPACK; } else if (eCall =3D=3D STAT_GET_KEY) { @@ -720,35 +715,25 @@ statGet(sql_context * context, int argc, sql_value = ** argv) p->iGet++; break; } - } - - int i; - - size_t size =3D mp_sizeof_array(p->nCol); - for (i =3D 0; i < p->nCol; ++i) { - size +=3D mp_sizeof_uint(aCnt[i]); - } - - char *z_ret =3D sqlMallocZero(size); - if (z_ret =3D=3D 0) { - sql_result_error_nomem(context); - } else { - char *z =3D z_ret; - z =3D mp_encode_array(z, p->nCol); - for (i =3D 0; i < p->nCol; i++) { - z =3D mp_encode_uint(z, aCnt[i]); } - const char *b =3D z_ret; - assert(mp_check(&b, z) =3D=3D 0); - (void) b; - assert(z_ret !=3D z); + size_t size =3D mp_sizeof_array(p->nCol); + for (int i =3D 0; i < p->nCol; ++i) + size +=3D mp_sizeof_uint(aCnt[i]); =20 - sql_result_blob(context, z_ret, size, sql_free); + char *mp_stat4 =3D malloc(size); + if (mp_stat4 =3D=3D NULL) { + sql_result_error_nomem(context); + return; + } + char *data =3D mp_stat4; + data =3D mp_encode_array(data, p->nCol); + for (int i =3D 0; i < p->nCol; i++) + data =3D mp_encode_uint(data, aCnt[i]); + assert(data =3D=3D mp_stat4 + size); + sql_result_blob(context, mp_stat4, size, free); context->pOut->flags|=3D MEM_Subtype; context->pOut->subtype =3D SQL_SUBTYPE_MSGPACK; } - -} #ifndef SQL_DEBUG UNUSED_PARAMETER(argc); #endif @@ -1188,13 +1173,20 @@ struct analysis_index_info { uint32_t index_count; }; =20 -#define KW_UNORDERED 0x01 -#define KW_NOSKIPSCAN 0x02 /** - * The first argument points to a msg_pack array + * Tokens which may present at array containing statistics. + * They can turn on/off certain optimizations and help query + * planner. To be extended. + */ +enum { + UNORDERED_HINT_TK =3D 0x1, + NOSKIPSCAN_HINT_TK =3D 0x2, +}; + +/** + * The first argument points to a msgpack array * containing a list of integers. Load the first * stat_size of these into the output arrays. - * keywords_info needed for keywords encoding/decoding. * * @param stat_array MP_ARRAY containing array of integers. * @param stat_size Size of input array (not counting the keywords). @@ -1204,12 +1196,24 @@ struct analysis_index_info { */ static void decode_stat_array(const char *stat_array, int stat_size, tRowcnt = *stat_exact, - LogEst *stat_log, uint8_t keywords_info) { + LogEst *stat_log, uint8_t *keywords_mask) +{ const char *z =3D stat_array; - uint32_t array_size =3D mp_decode_array(&z); - if (z =3D=3D NULL) + if (z =3D=3D NULL || mp_typeof(*z) !=3D MP_ARRAY) + return; + int array_size =3D mp_decode_array(&z); + /* + * Number of entries in array should be greater or equal + * to given size. If it greater, then it could contain + * hint tokens to enable/disable certain optimizations. + * But in case it is less, it is likely to be malformed + * data, so it makes no sense to continue processing. + */ + if (array_size < stat_size) return; for (int i =3D 0; i < stat_size; i++) { + if (mp_typeof(*z) !=3D MP_UINT) + return; tRowcnt v =3D (tRowcnt) mp_decode_uint(&z); if (stat_exact !=3D NULL) stat_exact[i] =3D v; @@ -1218,20 +1222,18 @@ decode_stat_array(const char *stat_array, int = stat_size, tRowcnt *stat_exact, } =20 /* Keywords processing if needed. */ - if (keywords_info !=3D 0) { + if (keywords_mask !=3D NULL) { + *keywords_mask =3D 0; uint32_t keyword_count =3D array_size - stat_size; - if (keyword_count > 0) { - while (keyword_count) { - const char *sval; - uint32_t sval_len; - sval =3D mp_decode_str(&z, &sval_len); - if (!sql_stricmp(sval, "unordered")) { - keywords_info |=3D KW_UNORDERED; - } else if (!sql_stricmp(sval, = "noskipscan")) { - keywords_info |=3D = KW_NOSKIPSCAN; - } - keyword_count--; - } + while (keyword_count-- > 0) { + uint32_t sval_len; + if (mp_typeof(*z) !=3D MP_STR) + return; + const char *sval =3D mp_decode_str(&z, = &sval_len); + if (strncmp(sval, "unordered", 9) =3D=3D 0) + *keywords_mask |=3D UNORDERED_HINT_TK; + else if (strncmp(sval, "noskipscan", 10) =3D=3D = 0) + *keywords_mask |=3D NOSKIPSCAN_HINT_TK; } } } @@ -1307,19 +1309,11 @@ analysis_loader(void *data, int argc, char = **argv, char **unused) diag_set(OutOfMemory, stat1_size, "region", = "tuple_log_est"); return -1; } - uint8_t keywords_info =3D 0; + uint8_t keywords_mask =3D 0; decode_stat_array(argv[2], column_count, stat->tuple_stat1, - stat->tuple_log_est, keywords_info); - - if ((keywords_info & KW_UNORDERED) =3D=3D 0) - stat->is_unordered =3D false; - else - stat->is_unordered =3D true; - if ((keywords_info & KW_NOSKIPSCAN) =3D=3D 0) - stat->skip_scan_enabled =3D true; - else - stat->skip_scan_enabled =3D false; - + stat->tuple_log_est, &keywords_mask); + stat->is_unordered =3D (keywords_mask & UNORDERED_HINT_TK); + stat->skip_scan_enabled =3D ! (keywords_mask & = NOSKIPSCAN_HINT_TK); return 0; } =20 @@ -1524,11 +1518,11 @@ load_stat_from_space(struct sql *db, const char = *sql_select_prepare, = &stat->samples[stats[current_idx_count].sample_count]; =20 decode_stat_array((char *)sql_column_text(stmt, 2), - column_count, sample->eq, 0, 0); + column_count, sample->eq, NULL, NULL); decode_stat_array((char *)sql_column_text(stmt, 3), - column_count, sample->lt, 0, 0); + column_count, sample->lt, NULL, NULL); decode_stat_array((char *)sql_column_text(stmt, 4), - column_count, sample->dlt, 0, 0); + column_count, sample->dlt, NULL, = NULL); /* Take a copy of the sample. */ sample->key_size =3D sql_column_bytes(stmt, 5); sample->sample_key =3D region_alloc(&fiber()->gc, diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result index b622a529d..009fa6488 100644 --- a/test/box-py/bootstrap.result +++ b/test/box-py/bootstrap.result @@ -4,7 +4,7 @@ box.internal.bootstrap() box.space._schema:select{} --- - - ['max_id', 511] - - ['version', 2, 1, 1] + - ['version', 2, 1, 2] ... box.space._cluster:select{} > diff --git a/test/sql-tap/analyze1.test.lua = b/test/sql-tap/analyze1.test.lua > index cc1259314..e6d512f97 100755 > --- a/test/sql-tap/analyze1.test.lua > +++ b/test/sql-tap/analyze1.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test =3D require("sqltester") > -test:plan(38) > +test:plan(39) >=20 > test:do_execsql_test( > - "analyze-4.1", > + "analyze-4.1.1", > [[ > DELETE FROM "_sql_stat1"; > - INSERT INTO "_sql_stat1" VALUES('t4', 't4i1', 'nonsense'); > - INSERT INTO "_sql_stat1" VALUES('t4', 't4i2', = '432653287412874653284129847632'); > + ]], { > + -- > + -- > + }) > + > +_sql_stat1 =3D box.space[box.schema.SQL_STAT1_ID] You can simply do this: _sql_stat1 =3D box.space._sql_stat1 > +_sql_stat1:insert{'t4', 't4i1', {'nonsense'}} > +_sql_stat1:insert{'t4', 't4i2', {432653287412874653284129847632}} Firstly, it would be better if each test-related thing would be placed in scope of test function. So, lets move these insertions to test:do_test func (you can check out examples in sql-tap/lua-tables.test.lua for instance). Secondly, simple insertion to _sql_stat spaces doesn=E2=80=99t trigger update of statistics in in-memory structs. In other words, these tests check nothing (and without your patch as well). To make these tests work, you should force statistics loading with instance reload. I am not sure if this possible in tap suite, so probably you should move these tests to sql/ suite. > +function sum_table(t) Please, provide brief comment to functions. > + local res =3D 0 > + for k, v in pairs(t) do > + res =3D res + v If you don=E2=80=99t need one of params, just skip it: for _, v in pairs(t) do =E2=80=A6 Also, you can use built-in facilities: fun =3D require('fun') fun.iter({1,2,3,4,5,6,7,8,9,10}):sum() > + end > + return res > +end > + > +function get_tuples_where_order_by_limit(order_by, order, limit) > + space =3D box.space[box.schema.SQL_STAT4_ID] > + t =3D {} > + for k, v in space:pairs() do > + table.insert(t, v) > + end > + > + local i > + local count Always initialise variables, it is best practice. > + local where =3D {tbl =3D "T1", idx =3D 'I1'} > + if where ~=3D 0 then This is always true. > + for k, v in pairs(where) do > + i =3D 1 > + for key, tuple in pairs(t) do > + tuple =3D t[i] > + if tuple[k] ~=3D v then > + t[i] =3D nil > + else > + count =3D i > + end > + i =3D i + 1 > + end > + end > + end Can=E2=80=99t read this code. Please, comment it at least. > + > + local compare > + if order =3D=3D 'asc' then > + compare =3D function(a, b) > + if sum_table(a[order_by]) <=3D sum_table(b[order_by]) = then > + return true > + end > + end > + else > + compare =3D function(a, b) > + if sum_table(a[order_by]) > sum_table(b[order_by]) then > + return true > + end > + end > + end > + > + table.sort(t, compare) > + > + if limit =3D=3D nil then > + limit =3D count > + end > + > + local ret =3D '' > + i =3D 1 > + while i <=3D limit do > + if i =3D=3D 1 then > + ret =3D tostring(t[i]) > + else > + ret =3D ret..' '..tostring(t[i]) > + end > + i =3D i + 1 > + end > + return ret > +end > + > +_sql_stat4 =3D box.space[box.schema.SQL_STAT4_ID] > + > +box.internal.sql_create_function("get_tuples_where_order_by_limit", = "TEXT", get_tuples_where_order_by_limit) > + > test:do_execsql_test( > "analyze-6.1.3", > [[ > - SELECT "tbl", "idx", "neq", "nlt", "ndlt" FROM = "_sql_stat4" where "tbl"=3D'T1' and "idx"=3D'I1' ORDER BY "nlt" LIMIT 1; > + SELECT get_tuples_where_order_by_limit('nlt', 'asc', 1); In this particular test(s), you pass the same limit and order by args. So I=E2=80=99d rather hardcode them to make function smaller and simpler.=20 > ]], { > -- > - "T1", "I1", "221 221 221 1", "0 0 0 10", "0 0 0 10" > + "[\'T1\', \'I1\', [221, 221, 221, 1], [0, 0, 0, 10], [0, 0, 0, = 10], !!binary lKF4oXmhego=3D]" > -- > }) >=20 >=20 >=20 >=20 > @@ -184,12 +187,12 @@ test:do_execsql_test( > ]], generate_tens(100)) >=20 > -- The first element in the "nEq" list of all samples should therefore = be 10. > --- =20 > + =20 Extra empty line. > test:do_execsql_test( > "3.3.2", > [[ > ANALYZE; > - SELECT lrange("neq", 1, 1) FROM "_sql_stat4" WHERE "idx" =3D = 'I2'; > + SELECT lrange(msgpack_decode_sample("neq"), 1, 1) FROM = "_sql_stat4" WHERE "idx" =3D 'I2'; > ]], generate_tens_str(24)) >=20 > = --------------------------------------------------------------------------= - > @@ -283,31 +286,100 @@ test:do_execsql_test( > -- > }) >=20 > +box.internal.sql_create_function("get_tuples_where_order_by_limit", = "TEXT", get_tuples_where_order_by_limit) > + > test:do_execsql_test( > 4.3, > [[ > - SELECT "neq", lrange("nlt", 1, 3), lrange("ndlt", 1, 3), = lrange(msgpack_decode_sample("sample"), 1, 3)=20 > - FROM "_sql_stat4" WHERE "idx" =3D 'I1' ORDER BY "sample" = LIMIT 16; > + SELECT get_tuples_where_order_by_limit('sample', 'asc', 16); > ]], { > -- <4.3> > - "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" > + "[\'T1\', \'I1\', [10, 10, 10], [0, 0, 0], [0, 0, 0], = !!binary kwAAoTA=3D], =E2=80=9C.. Firstly, it would be nice to see decoded sample; secondly - throw away = index/table. It would make diff look clearer. > + "[\'T1\', \'I1', [10, 10, 10], [10, 10, 10], [1, 1, 1], = !!binary kwEBoTE=3D], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [20, 20, 20], [2, 2, 2], = !!binary kwICoTI=3D], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [30, 30, 30], [3, 3, 3], = !!binary kwMDoTM=3D], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [40, 40, 40], [4, 4, 4], = !!binary kwQEoTQ=3D], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [50, 50, 50], [5, 5, 5], = !!binary kwUFoTU=3D], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [60, 60, 60], [6, 6, 6], = !!binary kwYGoTY=3D], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [70, 70, 70], [7, 7, 7], = !!binary kwcHoTc=3D], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [80, 80, 80], [8, 8, 8], = !!binary kwgIoTg=3D], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [90, 90, 90], [9, 9, 9], = !!binary kwkJoTk=3D], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [100, 100, 100], [10, 10, = 10], !!binary kwoKojEw], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [110, 110, 110], [11, 11, = 11], !!binary kwsLojEx], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [120, 120, 120], [12, 12, = 12], !!binary kwwMojEy], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [130, 130, 130], [13, 13, = 13], !!binary kw0NojEz], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [140, 140, 140], [14, 14, = 14], !!binary kw4OojE0], ".. > + "[\'T1\', \'I1\', [10, 10, 10], [150, 150, 150], [15, 15, = 15], !!binary kw8PojE1]" > -- > }) >=20 > test:do_execsql_test( > 4.4, > [[ > - SELECT "neq", lrange("nlt", 1, 3), lrange("ndlt", 1, 3), = lrange(msgpack_decode_sample("sample"), 1, 3)=20 > - FROM "_sql_stat4" WHERE "idx" =3D 'I1' ORDER BY "sample" DESC = LIMIT 2; > + SELECT get_tuples_where_order_by_limit('sample', 'desc', 2); > ]], { > -- <4.4> > - "2 1 1","295 296 296","120 122 125","201 4 h","5 3 1","290 = 290 291","119 119 120","200 1 b" > + "['T1', 'I1', [2, 1, 1], [295, 296, 296], [120, 122, 125], = !!binary k8zJBKFo], ".. > + "['T1', 'I1', [5, 3, 1], [290, 290, 291], [119, 119, 120], = !!binary k8zIAaFi]" > -- > }) >=20 > @@ -391,16 +463,41 @@ test:do_execsql_test( > INSERT INTO t1 VALUES(null, 4, 4); > INSERT INTO t1 VALUES(null, 5, 5); > ANALYZE; > - CREATE TABLE x1(tbl TEXT, idx TEXT , neq TEXT, nlt TEXT, ndlt = TEXT, sample BLOB, PRIMARY KEY(tbl, idx, sample)); > - INSERT INTO x1 SELECT * FROM "_sql_stat4"; > - DELETE FROM "_sql_stat4"; > - INSERT INTO "_sql_stat4" SELECT * FROM x1; > - ANALYZE; > ]]) >=20 > +function copy_tuples(from, to) > + for i,t in from:pairs() do > + to:insert(t) > + end > +end > + > +function prepare_to_6_2() > + local format =3D {} > + format[1] =3D {name=3D'tbl', type=3D'string'} > + format[2] =3D {name=3D'idx', type=3D'string'} > + format[3] =3D {name=3D'neq', type=3D'array'} > + format[4] =3D {name=3D'nlt', type=3D'array'} > + format[5] =3D {name=3D'ndlt', type=3D'array'} > + format[6] =3D {name=3D'sample', type=3D'scalar'} > + x1 =3D box.schema.space.create("x1", {engine =3D 'memtx', format = =3D format, field_count =3D 0}) > + x1:create_index('primary', {parts =3D {1, 'string', 2, 'string', = 6, 'scalar'}}) > + copy_tuples(_sql_stat4, x1) > +end > + > +prepare_to_6_2() Wrap this func in test-func call. > + > test:do_execsql_test( > 6.2, > [[ > + DELETE FROM "_sql_stat4"; > + ]]) > + > +copy_tuples(x1, _sql_stat4) > + > +test:do_execsql_test( > + 6.3, > + [[ > + ANALYZE; > SELECT * FROM t1 WHERE a =3D 'abc'; > ]]) >=20 > @@ -459,10 +556,19 @@ test:do_execsql_test( > -- -- > -- }) >=20 > +function update_stat_fields(stat, field_num, val) Why do you need this wrapper at all? > + for i,t in stat:pairs() do > + t =3D t:transform(3, 3) > + print(t) Debug print. > + stat:update(t, {{'=3D', field_num, val}}) > + end > +end > + > +update_stat_fields(_sql_stat4, 3, {0, 0, 0}) Wrap this func in test-func call. > + > test:do_execsql_test( > 7.3, > [[ > - UPDATE "_sql_stat4" SET "neq" =3D '0 0 0'; > ANALYZE; > SELECT * FROM t1 WHERE a =3D 1; > ]], { > @@ -471,11 +577,12 @@ test:do_execsql_test( > -- > }) >=20 > +box.sql.execute('ANALYZE=E2=80=99) Again you break philosophy of tap suite: wrap this analyze statement into one of testing funcs. >=20 > -- This is just for coverage.... > -test:do_execsql_test( > - 15.11, > - [[ > - ANALYZE; > - UPDATE "_sql_stat1" SET "stat" =3D "stat" || ' unordered'; > - ]]) > +box.sql.execute('ANALYZE') > +update_stat_fields(_sql_stat1, 3, {'unordered=E2=80=99}) Firstly, =E2=80=98unordered=E2=80=99 token should be the only member of array, it should be placed after original statistics. Secondly, without instance reloading all these insertions to stat space make no sense. > diff --git a/test/sql-tap/gh-3350-skip-scan.test.lua = b/test/sql-tap/gh-3350-skip-scan.test.lua > index 4cecfe081..eec09a546 100755 > --- a/test/sql-tap/gh-3350-skip-scan.test.lua > +++ b/test/sql-tap/gh-3350-skip-scan.test.lua > @@ -3,7 +3,7 @@ > -- gh-3350, gh-2859 >=20 > +local _sql_stat1 =3D box.space[box.schema.SQL_STAT1_ID] > +_sql_stat1:insert{'T1','T1ABC', {10000,5000,2000,10}} > + > +test:do_execsql_test( > + "skip-scan-1.4.2", > + [[ > ANALYZE t2; > SELECT a,b,c,d FROM t1 WHERE b=3D345; > ]], { > @@ -104,5 +113,4 @@ test:do_execsql_test( > } > ) >=20 > - Extra diff. > test:finish_test()