[tarantool-patches] [PATCH 0/2] Fix wrong field count in bytecode for ANALYZE

[tarantool-patches] [PATCH 0/2] Fix wrong field count in bytecode for ANALYZE

[Nikita Pettik]

Branch: https://github.com/tarantool/tarantool/tree/np/fix-analyze-makerecord

First patch fixes obvious bug in VDBE bytecode for ANALYZE SQL
statement. To prevent similar bugs in the future, second patch
explicitly sets field count for _sql_stat1 and _sql_stat4 system
spaces.

Nikita Pettik (2):
  sql: fix OP_MakeRecord argument of ANALYZE bytecode
  schema: add exact field count to SQL stat spaces

 src/box/bootstrap.snap                             | Bin 1831 -> 1840 bytes
 src/box/lua/upgrade.lua                            |  29 ++++++++++++++-
 src/box/sql/analyze.c                              |   2 +-
 test/box-py/bootstrap.result                       |   6 ++--
 test/box/access_misc.result                        |   4 +--
 ...{sql-statN-index-drop.result => analyze.result} |  17 +++++++++
 ...-statN-index-drop.test.lua => analyze.test.lua} |   6 ++++
 test/sql/upgrade.result                            |  39 +++++++++++++++++++--
 test/sql/upgrade.test.lua                          |  13 +++++++
 9 files changed, 107 insertions(+), 9 deletions(-)
 rename test/sql/{sql-statN-index-drop.result => analyze.result} (86%)
 rename test/sql/{sql-statN-index-drop.test.lua => analyze.test.lua} (91%)

-- 
2.15.1

[tarantool-patches] [PATCH 1/2] sql: fix OP_MakeRecord argument of ANALYZE bytecode

[Nikita Pettik]

Accidentally, wrong number of fields was passed to OP_MakeRecord opcode
which encodes tuple to be inserted to _sql_stat1 space: instead of three
fields (as format of space says), four were specified. Since this system
space doesn't feature exact field count, this insertion was successfully
processed. What is more, this additional field was invisible for SQL
means since it is out of space format (so it didn't get to resulting set
of SELECT * FROM _sql_stat1; queries). This patch fixes number of fields
to be encoded of this particular OP_MakeRecord.
---
 src/box/sql/analyze.c                                   |  2 +-
 .../sql/{sql-statN-index-drop.result => analyze.result} | 17 +++++++++++++++++
 .../{sql-statN-index-drop.test.lua => analyze.test.lua} |  6 ++++++
 3 files changed, 24 insertions(+), 1 deletion(-)
 rename test/sql/{sql-statN-index-drop.result => analyze.result} (86%)
 rename test/sql/{sql-statN-index-drop.test.lua => analyze.test.lua} (91%)

diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index ea5cbc39b..9d3df8a15 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -996,7 +996,7 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space)
 					     FIELD_TYPE_STRING,
 					     FIELD_TYPE_STRING,
 					     field_type_MAX };
-		sqlVdbeAddOp4(v, OP_MakeRecord, tab_name_reg, 4, tmp_reg,
+		sqlVdbeAddOp4(v, OP_MakeRecord, tab_name_reg, 3, tmp_reg,
 				  (char *)types, sizeof(types));
 		sqlVdbeAddOp4(v, OP_IdxInsert, tmp_reg, 0, 0,
 				  (char *)stat1, P4_SPACEPTR);
diff --git a/test/sql/sql-statN-index-drop.result b/test/sql/analyze.result
similarity index 86%
rename from test/sql/sql-statN-index-drop.result
rename to test/sql/analyze.result
index 760595188..42210ea13 100644
--- a/test/sql/sql-statN-index-drop.result
+++ b/test/sql/analyze.result
@@ -45,6 +45,23 @@ box.sql.execute("SELECT * FROM \"_sql_stat1\";")
   - ['T2', 'I1', '1 1']
   - ['T2', 'T2', '1 1']
 ...
+-- Make sure that tuples in spaces with statistics don't
+-- containt fields out of format.
+--
+box.space._sql_stat1:select()
+---
+- - ['T1', 'I1', '1 1']
+  - ['T1', 'T1', '1 1']
+  - ['T2', 'I1', '1 1']
+  - ['T2', 'T2', '1 1']
+...
+box.space._sql_stat4:select()
+---
+- - ['T1', 'I1', '1', '0', '0', !!binary kQI=]
+  - ['T1', 'T1', '1', '0', '0', !!binary kQE=]
+  - ['T2', 'I1', '1', '0', '0', !!binary kQI=]
+  - ['T2', 'T2', '1', '0', '0', !!binary kQE=]
+...
 -- Dropping an index.
 box.sql.execute("DROP INDEX i1 ON t1;")
 ---
diff --git a/test/sql/sql-statN-index-drop.test.lua b/test/sql/analyze.test.lua
similarity index 91%
rename from test/sql/sql-statN-index-drop.test.lua
rename to test/sql/analyze.test.lua
index 35f22910c..ba31d5fbc 100644
--- a/test/sql/sql-statN-index-drop.test.lua
+++ b/test/sql/analyze.test.lua
@@ -17,6 +17,12 @@ box.sql.execute("ANALYZE;")
 box.sql.execute("SELECT * FROM \"_sql_stat4\";")
 box.sql.execute("SELECT * FROM \"_sql_stat1\";")
 
+-- Make sure that tuples in spaces with statistics don't
+-- containt fields out of format.
+--
+box.space._sql_stat1:select()
+box.space._sql_stat4:select()
+
 -- Dropping an index.
 box.sql.execute("DROP INDEX i1 ON t1;")
 
-- 
2.15.1

[tarantool-patches] [PATCH 2/2] schema: add exact field count to SQL stat spaces

[Nikita Pettik]

As a rule, system spaces don't feature exact field count. There is a
reason for that: almost all of them have on replace triggers, which
extract data from tuple and verify it. However, _sql_stat1 and
_sql_stat4 spaces containing SQL specific statistics don't have such
triggers. Hence, one can insert tuple to this space with greater number
of fields, than format says. To prevent this, let's explicitly set exact
field count to number of fields in format.
---
 src/box/bootstrap.snap       | Bin 1831 -> 1840 bytes
 src/box/lua/upgrade.lua      |  29 ++++++++++++++++++++++++++++-
 test/box-py/bootstrap.result |   6 +++---
 test/box/access_misc.result  |   4 ++--
 test/sql/upgrade.result      |  39 +++++++++++++++++++++++++++++++++++++--
 test/sql/upgrade.test.lua    |  13 +++++++++++++
 6 files changed, 83 insertions(+), 8 deletions(-)

diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap
index 0bb446fb6903ac3ef630c419b909f7db3df0372a..920015b32da9c3e309ba6cb5af4caa1c7049827a 100644
GIT binary patch
delta 1836
zcmV+{2h;ec4zLc88Gke}FfC^=V>e+lWivT83Q2BrbYX5|WjY`*Vq`RBH)dolV`4Wl
zEi`0fWi2>iGhr=eH#B8AH(@q7Gh|^3RzqxWV{1AfdwmKD)w&D1%?7mq&i3~;xTOF9
z0000ewJ-euP-Qg$N=Hl&Nx&MZ0EmJh2!bF8hF~g#3d>-Ct$zXTCIY_*ogGZB3l?S0
zv`A!1vQm;X!`;3}iK7Sr06;-1K&mLsN`bnTU{D4DK@<UnOeqD~0M!8H09ih#ZqA~%
z92dQquZw)?I~!lGWVX_HcU}g;mdu~t`WAUx&+@pfQJ4XD?@N9T#ZGFTXW#1;=32CU
z`0BI|+xKqBZGYdonie!fKWsx0+$$T<{0y>`csHkR-#`Cnzu)^+|JnS~*k!FyN{yOy
zB>}H}HBOaW0ItsQwQmdWu*T{u{kwA;5cVul1>nQQj2Zn8^sp9dRMwF=uFi4E6Cpcv
zZp>&=($}D-O<70c>KrSr8pM3RtHy+Kb&f~W=)?Bswtq@k2SexN0u!XAi>%6*3tn`#
z0H`x4F1*9r&DZ1Q&35mTq`;zou<N^5H5cBYB`Jy&+t0<51Fp`I)x{gz&qYg8Qv)WR
z7F8}=QX2+bouk;FGx4;jvLvm6k-2P*b_@0mNj#JN{HL)kDUAXrE=xpb67>67DU<>N
zuFg^4&wn78%HHc)-u1}7Z|CIC)wng=zxu9mE&R@DPV|dC&+U!M9PoMU;xArv*&2;8
z&~Iyls&aL+itJhhJ^SUysv?b7`pYJ^q@@B}onv@gGu}gxc0Dkc;j8c0J|GE2ra|0q
z&Y)+BD2V`9=NLjZBWgs{gy8BNg=$Wn3D$w2xqpGte3<bt(_w}Ki(eK68<`rJ4Kf;H
zGQ?nrxln7#0)+`-p{1@wLn)~OSLaBYnJ}R&FZN(9mO%fz(<Tfxl65o`xH`w840=TO
z3q=jySYE#^lH(%%`-ZnKW0vQnYkYU-8uY9Zm6bw28LrN;b$R(|TGq3k*E87?0V9k|
z=zpB{^Wj-e^%-~Sw{x3&xFw=RBz)iRe*G)Uag?H=DYQonSLaB=zF$v^yW1P(DRFg<
zL#q|u-P-6*1g_3eq`^8DvuABI2Z9Ul5a$6`=eYWe0~J#A=5R<VkHtB1GGbGtBQi6D
z#t0!%BeDje8pPexqRLijtV01gIy&M)e}8awj-~m0zYA;8=7Xzq1W+f9b*LWuLy?oT
z0uK-8@OKgAc6<gm&BHk*h9#}B&ZI-*)^|xtb8vNzDpo31_~KcIMhvfmjv?>!y3tJ`
z4WD<hTvtw)#Xp3X<=uexbCK5hiz|}OSZBh))j8_;s4=6UO5>aTT)esY0aH{cjem7D
zH|S@Bt8@Hn1b_`0p#dwxW1};A?C{8-J27@((3}@JE^u1l=-}kQ%8Ft!Q7bsu5K6@<
z#R<jfgbBxlGMTU`VM8K;aH#QIWT*s25P$#$Ko9`Y1qU(ciXIYRz(|avFphy3h(izv
z0viWF01QG3G>BR-LXn>8u51GwX@5ZAZ@ur!JZis=fAE2U5nG_JyTYewk`lJB6RB(`
zDuF<i5iM*e^aCL^B{5ce$^sl4tCCd30QScLl%yJ3+3Y*@TAj)~$MQ6$dT^aKdr4V_
zKM}`avZ#<*34rTom7G!<Zy)U$YYIpRnx=XV#Q*~>gcyGI3cC*MjbmPI3V-p#6u@Jv
z)>*Z|4in#-H&2~s3qru4rG-WuS8YT<x`;^kf&nFoLr1fTim`TSv@=*Wj@xFlv0sj6
zGlRpevQGm6UG|;~O|6p|_mHvi^zb?1P6`iMxs66<IAWU?GR+=Ba6{h$cvv(AWeeyH
z`NrNbZ~Oc8Y_9DGtpv0{%ztc1-Ox0`-j7y7`W?TQfLkJ<dOY=GbPG_BCF@ItEaG$P
zfOh3yZmU^;viq_4JPiW>nanb#V=<AOA!zAGr{`{^3r6G;j<2*C{Yzf71u2%Er`5Ue
z86A5@g<ZgsmFgtv3ybrOMij?tn;eK4MvAs~STeP3_Qyo2=pR?(=YMekhQx4s+&Rza
zh0egGE~h>`<%_iA>G3>js_6qoUlZvTK`p{G;1|?rTdGEhXgMbg8c^bY!qNY&LhyYd
zpAaF@ZS6}7%3MQOI{LTJO617cQD|4(k*aHXA9YW7GdU#MreKWeV4nPa7iTwsf#Wnz
zSKH9#On09%gl9m}Gk@Zp5S<uuw^q(4Sqo<~=mfxJ@i9=a^rBw8ET~9fSRW<%&osj(
zH1rG)H`GbC&~opo^p=5L@{&HX|LIxvC}N4|je3%bt0<YY<J4|Chi~~4P^G#F%!>OD
z>VuKA@=@sw2`G!a`bSJ}4IoMjKrx1m8j@B*w0|6{3|7WNXC<lsI{RcwASot`*rA@c
avNNO`OT1tz9OiJyWmGL+<qp*lt?dfa%w>@P

delta 1827
zcmV+;2i*9u4yO)~8GkZ0H7#c}H)3WoGdVK~Np5p=VQyn(Iv`^*Gd5&pFkvk=W@R@m
zG%zzYEjT!4F)d?aGh{VnH8Wx{HDL-?Lu_wjYdRo%eF_TIx(m9^2Ce|k)s!0pr2qf`
z001bpFZ}>eEj0jYM??=v;25U>Fw8KA5aR@KMG#m>L*QW(A%7yj2IH-OV2p`GQD8SB
zQ<9aEzBH}eU*u^eqq>gJE=JE6+w)C1z1FnIIjXms%tRqmN&(pb)d1!IBbLvpo3p4b
z$3!pY+ag{1#zxmGiG3RH&dZ<IlG)Q)&mu4DR~@%A`ZD0|eaX+E*h$TE%zOP|u0`61
zt}gSi{qBaG_J6FaWj{gG!!{Jb{n32pXK<yYyE#?+{`o!Y{ob#7&t{j#En9_BTGWIi
z32^PJajM(`Y;}&VeOY*ivsM4}@6KsHxU)kQfDaclV)Q+*!&;nCA#Sv7b&gA(2-$&i
zV@8XTz6P~y3UQ;Y&T-PJLCp8JW=!Z-=h%f7dD!+`R)6{8Ug(@!UVgZAS1>>+ZNbR`
zpw6Eex9|>YH{XtzHQT#Sk^+hT!LINAW-Yu!OH$Mbwx5e92V0%vs*4x4pNp2HrUpzp
zEvj0!q%;h+I!CcTXVPg=Wl1{oBGj@o$}QM8Bymir=RJ*WNo5o`Y1ttflb~PEN}&`G
zY;}(Met!nRRPJ2A>TXB&eLE+6uEwm`{_nfSv+z5YIngifJhwFpbAab@i@W%&WoNWx
zpkLMoRpocHitJhhJL~1gDk6<P{bdtdQc{7f&M~~K81Eq{yB(Oz(AD>A9*~40q(R(o
z&R}PU=!n2pXC@OgC1^+h(Ttc8F%x2|a|D_>aet;42STRiCG#Q1LrjMl4l90F6lh>(
zU^c*LC}k*NC|#(PWP!ebu22$3lA&}|VXJc-#mpBFmKS%h7E7Q1-DUHI771}O6t+4?
zq6~IJ*9%1r)>d7=ERtg){riSBFXL6`q-%V4=Njy+5|x!gJ(#V|k#%|LX<62>o!2p;
z5r0M|bS~@ha4e^K?9^}PHt%pdM2kr9zTf@&Kg)3xqM#|XM`^2bBwycer^VfEjp~rL
zI>(^z!@FA=&55wpIf}AZ=VI<Gjov_L;T_sM*y<d=4;51M-fT#!j<q>*GD1^?BN8)&
zmV|6|j&($f$Qp!d5cf`tDq5v54h5u3mwzr==#Q<=ku;z0Z(%Lcd~9`&0P3VL4%K6S
zC~}fk;L+h6{wAW^j?dthbvUQQu%s2nnRIB(@-9hfj;+p7#iv?@E}n5{#OONc81g)>
z8{HJr;Q7|7W#x2N{6lzGUCn1d7iFEl_>pwNI1`So&Jo8)jTi-08rSUSVogmCn17-|
zDU6%BK|LE=ougMHz$?fIjaLyJ8lBN2M@I(DiID??-n_VRVbj6}$L8i$RuqegTCu%?
zP%2I-PAE<%Og1J|fo-VqTx6&OMi77i1war0(FF%F=!ymsV6Z@pqcDzv7zjfUiXs~a
zKmZIv3N(mXFhZ*abuTo5t}r0bJb#VtMSW;5s(;`EhOL>ZG@V_Dw&p-8+e?Qb5ZH(U
z8w&cNkRT-mlY5E*j$JyRR7_U;;{~ix4OzC?q3X4Ia+*1cr!ninwax4~%hK_LeTB&v
zLjVBRs^4;QWQeYPv}dd-Dk13b>Y^0K&07c!{KzY;+^{zwba*L5k*TZ4R)2A5)rMV5
z1W#@}b)GGRnL+;xjWVv<D$TnPrhAM5C5b{upNYCzXQ?ndZ_JlrcixyU53||%^AMcJ
zXn-!gCygMQX9hfEOg&wHPV7l;!Y8+w%G6)<X<;+(Ard$AAApCZBTbsYJgW!(nR&|J
zKW8&<KWHVG1Y*2nb*s||dVfD!iR^d$j8txkAl2j9AER4<f-G5IreqOrTQ9UL|8iT+
z`jed*i{oi{_|G(#F*%Eg@(e*se+*q_EnS!*7j%53&FC-ksx8Q~^uVod^Bkx>ON?E8
zB&%*d-&__)6H<((ZgQw$7zx{+W69LE*&h?7B7a<sAC~}Z0mJEG=YKq-7ghsTx}5mh
zm2YW>?Z;EnR68??zGSv7f?8y1z%Mw_woZ)_#d6LLT2tbG!rMPqA;^9qOVp6)w)Uk3
zWj-M@9sOHqC33{?D9#~wr0H7TN8Lc)Ob&^*Dez)Cm?wW<i_>jj;1n9Cw{1vsrpxC{
z>={t>jCkioCWhRtm4EX|*239%3IO;oK41!#Ues%p1qCUL?4u+<m}YW<L(kyhjyf?G
zTJAly-7>J7ywZd0|9Dm~is&PHOHUGW6(y5)oZ3z2@GO5Ksw8hRS@Hb?aZr+0cGNIK
z43t=2$s?w>#u23tKrzBb1(Hfa^nV<p45fgF?oxj_`)Ep_UK{`xaiyL&vvbKES-e0i
R9JX=Dom4Gm<qp*lt?e55T{r*$

diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index c72711ff7..4d0435e8e 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -616,6 +616,32 @@ local function upgrade_to_2_1_0()
     upgrade_priv_to_2_1_0()
 end
 
+--------------------------------------------------------------------------------
+-- Tarantool 2.1.1
+--------------------------------------------------------------------------------
+
+local function upgrade_to_2_1_2()
+    local _sql_stat1 = box.space[box.schema.SQL_STAT1_ID]
+    local _sql_stat4 = box.space[box.schema.SQL_STAT4_ID]
+    local _sql_stat1_def = box.space._space:get(box.schema.SQL_STAT1_ID):totable()
+    local _sql_stat4_def = box.space._space:get(box.schema.SQL_STAT4_ID):totable()
+    -- Set exact field count as the number of fields in format.
+    -- It makes sense since in contrast to other system spaces,
+    -- there is no on replace triggers which can verify content
+    -- of tuples to be inserted.
+    _sql_stat1_def[5] = #_sql_stat1:format()
+    _sql_stat4_def[5] = #_sql_stat4:format()
+    -- Just in case, erase all statistics before update.
+    for _, t in _sql_stat1.index[0]:pairs() do
+        _sql_stat1:delete({t[1], t[2]})
+    end
+    for _, t in _sql_stat4.index[0]:pairs() do
+        _sql_stat4:delete({t[1], t[2], t[6]})
+    end
+    box.space._space:replace(_sql_stat1_def)
+    box.space._space:replace(_sql_stat4_def)
+end
+
 local function get_version()
     local version = box.space._schema:get{'version'}
     if version == nil then
@@ -643,7 +669,8 @@ local function upgrade(options)
         {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
         {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
         {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
-        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true}
+        {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true},
+        {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true}
     }
 
     for _, handler in ipairs(handlers) do
diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
index 3e4394557..0ba533596 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, 0]
+  - ['version', 2, 1, 2]
 ...
 box.space._cluster:select{}
 ---
@@ -73,9 +73,9 @@ box.space._space:select{}
         'type': 'unsigned'}]]
   - [340, 1, '_space_sequence', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'},
       {'name': 'sequence_id', 'type': 'unsigned'}, {'name': 'is_generated', 'type': 'boolean'}]]
-  - [348, 1, '_sql_stat1', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
+  - [348, 1, '_sql_stat1', 'memtx', 3, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
         'type': 'string'}, {'name': 'stat', 'type': 'string'}]]
-  - [349, 1, '_sql_stat4', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
+  - [349, 1, '_sql_stat4', 'memtx', 6, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
         'type': 'string'}, {'name': 'neq', 'type': 'string'}, {'name': 'nlt', 'type': 'string'},
       {'name': 'ndlt', 'type': 'string'}, {'name': 'sample', 'type': 'scalar'}]]
   - [356, 1, '_fk_constraint', 'memtx', 0, {}, [{'name': 'name', 'type': 'string'},
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 4ffeb386a..5449834da 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -813,9 +813,9 @@ box.space._space:select()
         'type': 'unsigned'}]]
   - [340, 1, '_space_sequence', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'},
       {'name': 'sequence_id', 'type': 'unsigned'}, {'name': 'is_generated', 'type': 'boolean'}]]
-  - [348, 1, '_sql_stat1', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
+  - [348, 1, '_sql_stat1', 'memtx', 3, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
         'type': 'string'}, {'name': 'stat', 'type': 'string'}]]
-  - [349, 1, '_sql_stat4', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
+  - [349, 1, '_sql_stat4', 'memtx', 6, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
         'type': 'string'}, {'name': 'neq', 'type': 'string'}, {'name': 'nlt', 'type': 'string'},
       {'name': 'ndlt', 'type': 'string'}, {'name': 'sample', 'type': 'scalar'}]]
   - [356, 1, '_fk_constraint', 'memtx', 0, {}, [{'name': 'name', 'type': 'string'},
diff --git a/test/sql/upgrade.result b/test/sql/upgrade.result
index 02ab9b42b..1b1bb4c8c 100644
--- a/test/sql/upgrade.result
+++ b/test/sql/upgrade.result
@@ -30,12 +30,12 @@ box.space._space.index['name']:get('_trigger')
 ...
 box.space._space.index['name']:get('_sql_stat1')
 ---
-- [348, 1, '_sql_stat1', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
+- [348, 1, '_sql_stat1', 'memtx', 3, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
       'type': 'string'}, {'name': 'stat', 'type': 'string'}]]
 ...
 box.space._space.index['name']:get('_sql_stat4')
 ---
-- [349, 1, '_sql_stat4', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
+- [349, 1, '_sql_stat4', 'memtx', 6, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
       'type': 'string'}, {'name': 'neq', 'type': 'string'}, {'name': 'nlt', 'type': 'string'},
     {'name': 'ndlt', 'type': 'string'}, {'name': 'sample', 'type': 'scalar'}]]
 ...
@@ -158,3 +158,38 @@ test_run:cmd('cleanup server upgrade')
 ---
 - true
 ...
+work_dir = 'sql/upgrade/2.1.0/'
+---
+...
+test_run:cmd('create server upgrade210 with script="sql/upgrade/upgrade.lua", workdir="' .. work_dir .. '"')
+---
+- true
+...
+test_run:cmd('start server upgrade210')
+---
+- true
+...
+test_run:switch('upgrade210')
+---
+- true
+...
+box.space._sql_stat1.field_count
+---
+- 3
+...
+box.space._sql_stat4.field_count
+---
+- 6
+...
+test_run:switch('default')
+---
+- true
+...
+test_run:cmd('stop server upgrade210')
+---
+- true
+...
+test_run:cmd('cleanup server upgrade210')
+---
+- true
+...
diff --git a/test/sql/upgrade.test.lua b/test/sql/upgrade.test.lua
index cd4dd3cca..9ca3f18e2 100644
--- a/test/sql/upgrade.test.lua
+++ b/test/sql/upgrade.test.lua
@@ -53,3 +53,16 @@ box.sql.execute("DROP TABLE T_OUT;")
 test_run:switch('default')
 test_run:cmd('stop server upgrade')
 test_run:cmd('cleanup server upgrade')
+
+work_dir = 'sql/upgrade/2.1.0/'
+test_run:cmd('create server upgrade210 with script="sql/upgrade/upgrade.lua", workdir="' .. work_dir .. '"')
+test_run:cmd('start server upgrade210')
+
+test_run:switch('upgrade210')
+
+box.space._sql_stat1.field_count
+box.space._sql_stat4.field_count
+
+test_run:switch('default')
+test_run:cmd('stop server upgrade210')
+test_run:cmd('cleanup server upgrade210')
-- 
2.15.1

[tarantool-patches] Re: [PATCH 2/2] schema: add exact field count to SQL stat spaces

[Vladislav Shpilevoy]

On that branch tests sql-tap/gh2548-select-compound-limit.test.lua
and sql-tap/gh2250-trigger-chain-limit.test.lua fail.

On 19/03/2019 02:51, Nikita Pettik wrote:
> As a rule, system spaces don't feature exact field count. There is a
> reason for that: almost all of them have on replace triggers, which
> extract data from tuple and verify it. However, _sql_stat1 and
> _sql_stat4 spaces containing SQL specific statistics don't have such
> triggers. Hence, one can insert tuple to this space with greater number
> of fields, than format says. To prevent this, let's explicitly set exact
> field count to number of fields in format.
> ---
>   src/box/bootstrap.snap       | Bin 1831 -> 1840 bytes
>   src/box/lua/upgrade.lua      |  29 ++++++++++++++++++++++++++++-
>   test/box-py/bootstrap.result |   6 +++---
>   test/box/access_misc.result  |   4 ++--
>   test/sql/upgrade.result      |  39 +++++++++++++++++++++++++++++++++++++--
>   test/sql/upgrade.test.lua    |  13 +++++++++++++
>   6 files changed, 83 insertions(+), 8 deletions(-)
> 

[tarantool-patches] Re: [PATCH 2/2] schema: add exact field count to SQL stat spaces

[n.pettik]

> On 21 Mar 2019, at 16:31, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> On that branch tests sql-tap/gh2548-select-compound-limit.test.lua
> and sql-tap/gh2250-trigger-chain-limit.test.lua fail.

Are you sure? Travis is green; on my local machine both
release and debug builds pass these tests as well (memtx
and vinyl cases). Could you check it again, or provide more
details concerning fails?

> On 19/03/2019 02:51, Nikita Pettik wrote:
>> As a rule, system spaces don't feature exact field count. There is a
>> reason for that: almost all of them have on replace triggers, which
>> extract data from tuple and verify it. However, _sql_stat1 and
>> _sql_stat4 spaces containing SQL specific statistics don't have such
>> triggers. Hence, one can insert tuple to this space with greater number
>> of fields, than format says. To prevent this, let's explicitly set exact
>> field count to number of fields in format.
>> ---
>>  src/box/bootstrap.snap       | Bin 1831 -> 1840 bytes
>>  src/box/lua/upgrade.lua      |  29 ++++++++++++++++++++++++++++-
>>  test/box-py/bootstrap.result |   6 +++---
>>  test/box/access_misc.result  |   4 ++--
>>  test/sql/upgrade.result      |  39 +++++++++++++++++++++++++++++++++++++--
>>  test/sql/upgrade.test.lua    |  13 +++++++++++++
>>  6 files changed, 83 insertions(+), 8 deletions(-)

[tarantool-patches] Re: [PATCH 2/2] schema: add exact field count to SQL stat spaces

[Vladislav Shpilevoy]

On 21/03/2019 18:22, n.pettik wrote:
> 
> 
>> On 21 Mar 2019, at 16:31, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>>
>> On that branch tests sql-tap/gh2548-select-compound-limit.test.lua
>> and sql-tap/gh2250-trigger-chain-limit.test.lua fail.
> 
> Are you sure? Travis is green; on my local machine both
> release and debug builds pass these tests as well (memtx
> and vinyl cases). Could you check it again, or provide more
> details concerning fails?

Hmm, I do not know, what it was, but now they pass :)

LGTM.

> 
>> On 19/03/2019 02:51, Nikita Pettik wrote:
>>> As a rule, system spaces don't feature exact field count. There is a
>>> reason for that: almost all of them have on replace triggers, which
>>> extract data from tuple and verify it. However, _sql_stat1 and
>>> _sql_stat4 spaces containing SQL specific statistics don't have such
>>> triggers. Hence, one can insert tuple to this space with greater number
>>> of fields, than format says. To prevent this, let's explicitly set exact
>>> field count to number of fields in format.
>>> ---
>>>   src/box/bootstrap.snap       | Bin 1831 -> 1840 bytes
>>>   src/box/lua/upgrade.lua      |  29 ++++++++++++++++++++++++++++-
>>>   test/box-py/bootstrap.result |   6 +++---
>>>   test/box/access_misc.result  |   4 ++--
>>>   test/sql/upgrade.result      |  39 +++++++++++++++++++++++++++++++++++++--
>>>   test/sql/upgrade.test.lua    |  13 +++++++++++++
>>>   6 files changed, 83 insertions(+), 8 deletions(-)
>