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 4977F26228 for ; Thu, 14 Jun 2018 15:27:25 -0400 (EDT) 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 k8AOruw9sNVS for ; Thu, 14 Jun 2018 15:27:25 -0400 (EDT) Received: from smtp56.i.mail.ru (smtp56.i.mail.ru [217.69.128.36]) (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 EE11D261ED for ; Thu, 14 Jun 2018 15:27:24 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v3 08/10] sql: new _trigger space format with space_id References: From: Vladislav Shpilevoy Message-ID: <41a4720e-ecf1-51c1-db01-f0c9ea69479d@tarantool.org> Date: Thu, 14 Jun 2018 22:27:22 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Kirill Shcherbatov , tarantool-patches@freelists.org Thanks for the fixes! See 2 comments below. Besides, I have pushed other review fixes as a separate commit. Please, look and squash. On 14/06/2018 20:32, Kirill Shcherbatov wrote: > As we would like to lookup triggers by space_id in future > on space deletion to delete associated _trigger tuples we > need to introduce new field space_id as secondary key. > > Part of #3273. > --- > src/box/bootstrap.snap | Bin 1698 -> 1704 bytes > src/box/lua/upgrade.lua | 4 +++ > src/box/schema_def.h | 6 ++++ > src/box/sql.c | 21 +++++++----- > src/box/sql/sqliteInt.h | 2 ++ > src/box/sql/trigger.c | 8 +++-- > test/app-tap/tarantoolctl.test.lua | 2 +- > test/box-py/bootstrap.result | 5 +-- > test/box/access_misc.result | 4 +-- > test/box/access_sysview.result | 4 +-- > test/box/alter.result | 1 + > test/sql/gh2141-delete-trigger-drop-table.result | 4 +-- > test/sql/gh2141-delete-trigger-drop-table.test.lua | 4 +-- > test/sql/persistency.result | 8 ++--- > test/sql/persistency.test.lua | 8 ++--- > test/sql/upgrade.result | 37 ++++++++++++++++++--- > test/sql/upgrade.test.lua | 9 ++++- > 17 files changed, 91 insertions(+), 36 deletions(-) > > diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap > index d271dca7bc6030f6a05c28c6385ed01a0a02f700..a8a00ec29e7106afbd06294cf6ffe291f90a2e10 100644 > GIT binary patch > delta 1694 > zcmV;P24VT44X6!}9)B}5W;ZrBISNT`b97;DV`VxZVq`I4G%`73EjBVRG%YkYH!&?Z > zW-vD`GG;kpI50CYF*i6k3RXjGZ)0mZAbWiZ3e~y`y3GbK0M4rK@!O>U00000D77#B > z08l+K04hVU4oT1$Hvs@&JireJkQ`J#QC_IkDFq@CiO~a`bblj zB$ELmxPvu%zAcout22V@@+qYh(*Vl=;{f7}eD(eMXFOG}!_0MA > zQ{^T>M0(5}i{c1=M)gI&wbZ2YVhW35sXv~Nz7( zf)$}az4fgVw||}m@{d+sDw2{7Ccw4Sd~Mvyo20RNsDF1}Gm^p2$O5>Qnnjr8NZ}$@ > zUp0bRm$XW>z1(Z5NvqXk=KEc=B-c_isUoTJ2ZUR*dM!1-%Q zOeK)R!6gtY@J}Yw2}73sotGj=gKaLA(km`OP_%4Waet%=mK`eqEDZ`TuDnT z;fvXoH(B&`@Q0+vqyMpMyhEBRZ}K~<*I;NrFFOX;Qj^t7JhY#e-&r}AB%m%=u5VUt > z7FT>m+wMGHamSAJ@%rPRMSLyuCDh+}quFt5>9Ozi}J*gB7TuV)jUmVAj > z2)>>LVt?@L8+U#JVVzsC{o!|=Y2 z%|a}$&aTT5{8ZxS)sfDj{+W^oG0DGKg&A-yHA&ox@lK915rbnvcwj(U)G7-cD|%E^ > zo)kPNs?Ls%4(bHT1gZpVW<3(r8*wc)FARw%qJKkmUU%X`a3DC7O^ywYkWE*^)qJ>? > znnE=dk;V@$}Q!}%H#W9P52AK>p7-TNQScutTXsm^)P+>w-D$EQFg<4-^ > zuBGP71PT+P@}*DK<;=+c?lS69k%%@L3D;6{V-bU73-g&u)G?peF(Oltn9gNDsvHwj > zJ%849-1#T^@Xx3Mfl+ zuBB!P`)XAuo_FfH9L~e26;A2It-MlqI+;o)4kttjMB$ilr%GYSQem5pkP1O65S{zO > z_V_^c=y=%d*mTU~s4+OMrKWwUNT zjmEXq1Yl_$+PoS2$&#P7f($T= z`V1LBw > zxYGG%KQC`?fW}mmT8Fk87Xe^M0)a@V;aFr43CsWhfB+}}@dXDd=ZXdrfUr1 z7=}Xtj6xGfKp-dx2{b^pU^HXwVBKS4p%&1X|4*aC=rB5r4iKkj+pXF1yo`v%b|Iu9 > zdofA^fdyFzLvnw7cVbd<_StLc@_#R^s5l<^p4W;`8)7qS9>v{|TchhZB_P^);=ymZ > zIl7E^Uvlc<%`Fj-7xknjySbbQ4f!DO#f(gG#AT(F2pzK&f)RTwwyKFODV_57>)62c > zcU8a_L{GIZbY%Pz#>pe6#^}gK1ydGiF#eR)Gjp;Gt+E+g;bR)c>^vjr(tmsM89goz > z1A&B6?YN%{OOg{CnHyC3+<)`5__2GC1PwVg^uSGQF#eR)Gjp;GO~6}!wtV)haNl!i > zV3weV8pz9g_D>aT=wIklyeHtL&G749Jsmxug;UC;VbMR~u&zFKv>o>O!QR-KmFgwy > z50A^=RP+dFcJSHQk?!c?`F}z%LBKyYQbqk3bpDiyd|?vZ^C0TiDxb zj|Ta;4hk`H!92Ly7#DV+#&F9_ZH8^mDu2j1dRS2>UXm?dxrK1*3vmvJgznO%+@MU9 > z&FbjovR)z~e@Aib@JDa0jX`Rx;muN>Oq+mTOc!e{6IVo}y#OjS&VNwbdcc{Hozr7r > zK=WtO9rs}jIa>?olPm@&{Ada=x%f*h`1( zxw&QFHhIOe`ahl%;E{JMQbFlmJc$@rD}kiHR_%KIk*T>JWoBLv1#B$^hO@mwpbE8P > oJi~Ha`5lx^HFfBzk{A9#Sl5<&4hy7?2*p>)dXg|iIn@xY?G8vRL;wH) > > delta 1688 > zcmV;J250%G4WbQ@9)CA7F*0H`F*r5~Np5p=VQyn(Iv{3aVKy^mG-EAfIAUZiG-EX~ > zEn+h@W-VnnF)%n{F)}n_H#iDbLu_wjYdRo%F*+bHeF_TIx(m9^1|tB@X~^8mr2qf` > z001bpFZ}>e)i411LZl8!&{$0YfIW;-52I0K5Pk_XKqcbbL4TXX{Q$^sCLS zrlck%?V2P)zDy=66lajMGo8{c399v6>?$}HOBd`M$VQ%~lv2_F$pGX4H4)bLYoM_F > zUb8aS z+KaGG6N(L)L4V!#uJXFRg?VSAEfnbpR|;V3Bw_oy@)m8a&eFd-zY)o}GpYb=o#YTD > zI8eBl(f>l6^=P9+oQt(}l9N^)X1@PrGh*u`lPJ=cyP)f;QClZ@Z*){1>T_(q9F%FL > zljE6A_=ID!QT@tvJ?z<_R2T!6{hf~>w#B#>I;WKup?@S7vOr0qa)qY~fI5W&d5bCs > zwodX{59U|iVo}%e2l2wA|FG-3S+-Z+Vpq~s1KEB)q8QjZ$yOh)Y(F2nlAc zU5(N#uyvASe{KZpan+S{Mgb9*U|+G!GoqmXbna?Y1_6>+XGCKT^!s&9C z!_WgU?0;MM; z+Tp6ky5OqpdJJ**fuC1LI zDSA+}I6-%Kc6L-IR3%g-VzrtYF;gOJourKwNq>bvsZc4D&MD^v;)FwULt`^UgPJ+b > ze6V$rLo+p;DaLt_=@66Q5JM9)6SJYkZ;OHkm<%u&U@p8^c;Q}TYAscuu^}n66jP!= > zsSAv)lXRIdV?$GZ{K z8Gq>Y8LHpTJMf2hM)XJN`+oQ9XLUR?IV$3W`b4mGlCgfV9(V7GXb%KiC&^;v-MbrY > z@xazejJrfK>QF > zeeBpCwTFjl2TBJ`4h~0*4}G~|>m;iSMSm)*4XVT3u^v~qN@ZNCMp9Ce6-vX_NtWjG > z{jaRY+YDPL37}3Y<5DvAiz6>-g%~)4;_oABVLTXa8qT0MiZxng-06kRuJ4wV_SiZ} > z60B8AgmIAP#R}Y^Mvn)A$>=BX1<=1*uPCV7;wJI7a1oLHe57^$BderS#+_W)I)6zX > z957}yT zFpPm1hGPJXLIX!YASeh4G(fdrG$SlvT|;7_7Qm4EO(KKHATo#yVAEsm)_AK7Gb$3> > zC6gNKVl4^83lbp=-T(O3#iSJ3$A7HF$X~Uh;uz#)u2r8lgl6Wrk2@AyOZRCSK~7uM > z8$U_)nFHdHKZX;08u1@rJKYX)NY>k3oEKG_B~q0v=#;`(!Pa9YNfM{b`_(mY`A-$} > z1<~_Xf*rb*hH+Bl7BE_-QC}nl4hTGi zZ;bSFGLlGSl5>kVQF_`iE$e&_$E6`_1|3L&1_U0$aafELfe;}4pUpXYH+svoBAh#P > z@dgrmFJqZ*`1%)yE8f3x#FG1UA^0m+L$`uR>aa-u*&eP2ar7N_#ALTfgH?K#gg?kF > ze^a3!qGf?*Vh-wjTpt$XLw|CdekL=j`lI~c`SthmqPlZpGpu>+RNndo^%05{*RnB2 > zFO7q%8RNhP&=_jtJYg6`dMQfI(FYbO;yfkH?`|Oo{6Yv#k>qY&%Nvw|f>|B8+~_3| > zhVLkk4}WCVS{_LK8r}@$lW7Cs#dNOLFma;t)C)|ZaU8W>2Amn$Ie$I|1{9wb-EsNF > zklWfhpY&pIktb6C&Bb?W!Of5Q9&w?5gxQ?FWDwKz$4GP=9`2|!!?sp>C6rqRrY5g= > zslUN<5+2!(#TS%Lizk_JmHd$KZ`E$DKmVEmQD)}$P@7gzU^wj80=lYQ(=+Yk+TKB% > itF48e>+~O}byP9oun~gm$Q^t|sV4~omjl%ht?dd`FccL4 1. Please, do not send binary files. > diff --git a/test/sql/upgrade.result b/test/sql/upgrade.result > index dcdb689..882bdd0 100644 > --- a/test/sql/upgrade.result > +++ b/test/sql/upgrade.result > @@ -79,10 +82,33 @@ box.space._space.index['name']:get('T_OUT') > - [514, 1, 'T_OUT', 'memtx', 1, {'sql': 'CREATE TABLE t_out(x INTEGER PRIMARY KEY)'}, > [{'type': 'integer', 'nullable_action': 'abort', 'name': 'X', 'is_nullable': false}]] > ... > -box.space._trigger:get('T1T') > +t1t = box.space._trigger:get('T1T') > +--- > +... > +t2t = box.space._trigger:get('T2T') > +--- > +... > +t1t.name > +--- > +- T1T > +... > +t1t.opts > --- > -- ['T1T', {'sql': 'CREATE TRIGGER t1t AFTER INSERT ON t BEGIN INSERT INTO t_out VALUES(1); > - END;'}] > +- {'sql': 'CREATE TRIGGER t1t AFTER INSERT ON t BEGIN INSERT INTO t_out VALUES(1); > + END;'} > +... > +t2t.name > +--- > +- T2T > +... > +t2t.opts > +--- > +- {'sql': 'CREATE TRIGGER t2t AFTER INSERT ON t BEGIN INSERT INTO t_out VALUES(2); > + END;'} > +... > +assert(t1t.id == t2t.id) > +--- > +- true 2. Please, add a test that t1t == box.space.T.id. Your current test would pass even if t1t.id == t2t.id == -100. > ... > box.sql.execute("INSERT INTO T VALUES(1);") > ---