Re: [sqlite] SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)
Hi Jan, Thanks for trying out my patch, and for your improvement. If I may suggest one thing: change the 'for alignment' comments to something more informative, e.g., 'please keep this line for SQLITE_ENABLE_UPDATE_DELETE_LIMIT debugging'. I also toyed with the idea of running lemon with '-l' both times, so that we rid ourselves of the #line's altogether. I couldn't decide if this was a good idea, so I left the #line's in. However, looking at the the parse.c code JUST NOW, I realized that there is something more fundamentally wrong: The '#line ... parse.c' directives should really be '#line ... parse.y'! I was wondering why I was getting inconsistent line numbers for the 2 invocations of lemon, since we're using the same parse.y both times, but did not manage to put the pieces together until just now. :-) Here is what I would suggest, and please let me know what you think. We do the change it 3 steps: (1) commit my patch, together with your patch below; (2) fix lemon so that it correctly outputs '#line ... parse.y' instead 'parse.c' ; (3) remove your patch below, since at that point it might become confusing to developers. Another sequence might be: (1) commit my patch, together with -l options provided for the lemon invocations; (2) fix lemon so that it correctly outputs '#line ... parse.y' instead 'parse.c' ; (3) remove the '-l' options from the lemon invocations. I can definitely do all the work, unless you (Jan) would like to jump in. SQLite maintainers, please feel free to opine anytime. :-) I see that a new release of SQLite is being put together, so all the fixes we're discussing here should probably go in afterwards. --Zem -Original Message- From: Jan Nijtmans [mailto:jan.nijtm...@gmail.com] Sent: Wednesday, 8 February 2017 2:58 To: Ziemowit Laski Cc: sqlite-users@mailinglists.sqlite.org Subject: Re: SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: [sqlite] Patch Etiquette) 2017-02-06 23:25 GMT+01:00 Ziemowit Laski: > Here is my approach to the SQLITE_ENABLE_UPDATE_DELETE_LIMIT problem. > [If the attachment does not arrive intact, please let me know.] Hi Ziemowit, I tried your patch, and it works fine! Below is an additional patch, which makes the resulting amalgamation much smaller (from 202734 to 202415 lines, while the original amalgamation was 201287 lines). The reason for this reduction is that your amalgamation contains 321 segments which look like: #ifndef SQLITE_ENABLE_UPDATE_DELETE_LIMIT #line 3288 "parse.c" #else /* SQLITE_ENABLE_UPDATE_DELETE_LIMIT */ #line 3290 "parse.c" #endif /* SQLITE_ENABLE_UPDATE_DELETE_LIMIT */ This is not really useful, since the only difference is the line number. If both "parse.c" versions contain the same line numbering, that helps in the diff size. Thank you for your idea! Regards, Jan nijtmans Index: src/parse.y == --- src/parse.y +++ src/parse.y @@ -748,10 +748,11 @@ %ifndef SQLITE_ENABLE_UPDATE_DELETE_LIMIT cmd ::= with(C) DELETE FROM fullname(X) indexed_opt(I) where_opt(W). { sqlite3WithPush(pParse, C, 1); sqlite3SrcListIndexedBy(pParse, X, ); sqlite3DeleteFrom(pParse,X,W); + /* for alignment */ } %endif %type where_opt {Expr*} %destructor where_opt {sqlite3ExprDelete(pParse->db, $$);} @@ -776,10 +777,11 @@ where_opt(W). { sqlite3WithPush(pParse, C, 1); sqlite3SrcListIndexedBy(pParse, X, ); sqlite3ExprListCheckLength(pParse,Y,"set list"); sqlite3Update(pParse,X,Y,W,R); + /* for alignment */ } %endif %type setlist {ExprList*} %destructor setlist {sqlite3ExprListDelete(pParse->db, $$);} ___ sqlite-users mailing list sqlite-users@mailinglists.sqlite.org http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Re: [sqlite] SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)
2017-02-06 23:25 GMT+01:00 Ziemowit Laski: > Here is my approach to the SQLITE_ENABLE_UPDATE_DELETE_LIMIT problem. [If > the attachment does not arrive intact, please let me know.] Hi Ziemowit, I tried your patch, and it works fine! Below is an additional patch, which makes the resulting amalgamation much smaller (from 202734 to 202415 lines, while the original amalgamation was 201287 lines). The reason for this reduction is that your amalgamation contains 321 segments which look like: #ifndef SQLITE_ENABLE_UPDATE_DELETE_LIMIT #line 3288 "parse.c" #else /* SQLITE_ENABLE_UPDATE_DELETE_LIMIT */ #line 3290 "parse.c" #endif /* SQLITE_ENABLE_UPDATE_DELETE_LIMIT */ This is not really useful, since the only difference is the line number. If both "parse.c" versions contain the same line numbering, that helps in the diff size. Thank you for your idea! Regards, Jan nijtmans Index: src/parse.y == --- src/parse.y +++ src/parse.y @@ -748,10 +748,11 @@ %ifndef SQLITE_ENABLE_UPDATE_DELETE_LIMIT cmd ::= with(C) DELETE FROM fullname(X) indexed_opt(I) where_opt(W). { sqlite3WithPush(pParse, C, 1); sqlite3SrcListIndexedBy(pParse, X, ); sqlite3DeleteFrom(pParse,X,W); + /* for alignment */ } %endif %type where_opt {Expr*} %destructor where_opt {sqlite3ExprDelete(pParse->db, $$);} @@ -776,10 +777,11 @@ where_opt(W). { sqlite3WithPush(pParse, C, 1); sqlite3SrcListIndexedBy(pParse, X, ); sqlite3ExprListCheckLength(pParse,Y,"set list"); sqlite3Update(pParse,X,Y,W,R); + /* for alignment */ } %endif %type setlist {ExprList*} %destructor setlist {sqlite3ExprListDelete(pParse->db, $$);} ___ sqlite-users mailing list sqlite-users@mailinglists.sqlite.org http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Re: [sqlite] SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)
Hello, I checked the message archive and it appears that the attachment got stripped from my previous message. So I'm plastering it inline below. Thank you, --Zem diff -C5 sqlitesrc2/sqlite-src-3160200/main.mk sqlitesrc3/sqlite-src-3160200/main.mk *** sqlitesrc2/sqlite-src-3160200/main.mk 2017-01-06 08:52:10.0 -0800 --- sqlitesrc3/sqlite-src-3160200/main.mk 2017-02-06 14:04:14.725992100 -0800 *** *** 626,642 opcodes.h:parse.h $(TOP)/src/vdbe.c $(TOP)/tool/mkopcodeh.tcl cat parse.h $(TOP)/src/vdbe.c | \ tclsh $(TOP)/tool/mkopcodeh.tcl >opcodes.h # Rules to build parse.c and parse.h - the outputs of lemon. # parse.h: parse.c parse.c: $(TOP)/src/parse.y lemon $(TOP)/tool/addopcodes.tcl cp $(TOP)/src/parse.y . rm -f parse.h ! ./lemon -s $(OPTS) parse.y mv parse.h parse.h.temp tclsh $(TOP)/tool/addopcodes.tcl parse.h.temp >parse.h sqlite3.h:$(TOP)/src/sqlite.h.in $(TOP)/manifest.uuid $(TOP)/VERSION $(TOP)/ext/rtree/sqlite3rtree.h tclsh $(TOP)/tool/mksqlite3h.tcl $(TOP) >sqlite3.h --- 626,654 opcodes.h:parse.h $(TOP)/src/vdbe.c $(TOP)/tool/mkopcodeh.tcl cat parse.h $(TOP)/src/vdbe.c | \ tclsh $(TOP)/tool/mkopcodeh.tcl >opcodes.h # Rules to build parse.c and parse.h - the outputs of lemon. + # NB: To build parse.c, we run lemon twice - once with + # -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT and once without - + # and then diff the results. This way, SQLITE_ENABLE_UPDATE_DELETE_LIMIT + # can be used directly with the amalgamation. # parse.h: parse.c parse.c: $(TOP)/src/parse.y lemon $(TOP)/tool/addopcodes.tcl cp $(TOP)/src/parse.y . rm -f parse.h ! ./lemon -s $(OPTS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) \ !parse.y ! mv parse.c parse.c.1.temp ! mv parse.out parse.out.1.temp ! ./lemon -s $(OPTS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) \ ! -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT parse.y ! mv parse.c parse.c.2.temp ! mv parse.out parse.out.2.temp ! -diff -d -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT parse.c.1.temp parse.c.2.temp >parse.c mv parse.h parse.h.temp tclsh $(TOP)/tool/addopcodes.tcl parse.h.temp >parse.h sqlite3.h:$(TOP)/src/sqlite.h.in $(TOP)/manifest.uuid $(TOP)/VERSION $(TOP)/ext/rtree/sqlite3rtree.h tclsh $(TOP)/tool/mksqlite3h.tcl $(TOP) >sqlite3.h diff -C5 sqlitesrc2/sqlite-src-3160200/Makefile.in sqlitesrc3/sqlite-src-3160200/Makefile.in *** sqlitesrc2/sqlite-src-3160200/Makefile.in 2017-01-06 08:52:10.0 -0800 --- sqlitesrc3/sqlite-src-3160200/Makefile.in 2017-02-06 14:06:04.625426600 -0800 *** *** 940,956 opcodes.h:parse.h $(TOP)/src/vdbe.c $(TOP)/tool/mkopcodeh.tcl cat parse.h $(TOP)/src/vdbe.c | $(TCLSH_CMD) $(TOP)/tool/mkopcodeh.tcl >opcodes.h # Rules to build parse.c and parse.h - the outputs of lemon. # parse.h: parse.c parse.c: $(TOP)/src/parse.y lemon$(BEXE) $(TOP)/tool/addopcodes.tcl cp $(TOP)/src/parse.y . rm -f parse.h ! ./lemon$(BEXE) $(OPT_FEATURE_FLAGS) $(OPTS) parse.y mv parse.h parse.h.temp $(TCLSH_CMD) $(TOP)/tool/addopcodes.tcl parse.h.temp >parse.h sqlite3.h:$(TOP)/src/sqlite.h.in $(TOP)/manifest.uuid $(TOP)/VERSION $(TCLSH_CMD) $(TOP)/tool/mksqlite3h.tcl $(TOP) >sqlite3.h --- 940,969 opcodes.h:parse.h $(TOP)/src/vdbe.c $(TOP)/tool/mkopcodeh.tcl cat parse.h $(TOP)/src/vdbe.c | $(TCLSH_CMD) $(TOP)/tool/mkopcodeh.tcl >opcodes.h # Rules to build parse.c and parse.h - the outputs of lemon. + # NB: To build parse.c, we run lemon twice - once with + # -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT and once without - + # and then diff the results. This way, SQLITE_ENABLE_UPDATE_DELETE_LIMIT + # can be used directly with the amalgamation. # parse.h: parse.c parse.c: $(TOP)/src/parse.y lemon$(BEXE) $(TOP)/tool/addopcodes.tcl cp $(TOP)/src/parse.y . rm -f parse.h ! ./lemon$(BEXE) $(OPT_FEATURE_FLAGS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) \ !$(OPTS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) parse.y ! mv parse.c parse.c.1.temp ! mv parse.out parse.out.1.temp ! ./lemon$(BEXE) $(OPT_FEATURE_FLAGS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) \ ! $(OPTS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) \ ! -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT parse.y ! mv parse.c parse.c.2.temp ! mv parse.out parse.out.2.temp ! -diff -d -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT parse.c.1.temp parse.c.2.temp >parse.c mv parse.h parse.h.temp $(TCLSH_CMD) $(TOP)/tool/addopcodes.tcl parse.h.temp >parse.h sqlite3.h:$(TOP)/src/sqlite.h.in $(TOP)/manifest.uuid $(TOP)/VERSION $(TCLSH_CMD) $(TOP)/tool/mksqlite3h.tcl $(TOP) >sqlite3.h diff
Re: [sqlite] SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)
Hello Jan + Maintainers, Here is my approach to the SQLITE_ENABLE_UPDATE_DELETE_LIMIT problem. [If the attachment does not arrive intact, please let me know.] It is different from Jan's in that it operates strictly on the Makefile level. The idea is simple: When compiling parse.y with lemon, we do it twice - once with -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT and once without, obtaining two temporary C parser files. Then, we diff the two files with the -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT option, producing a parse.c with SQLITE_ENABLE_UPDATE_DELETE_LIMIT if-defs. This parse.c is then sucked into the amalgamation, and now one can use SQLITE_ENABLE_UPDATE_DELETE_LIMIT directly with the amalgamation, and without needing a full source compile. Should you wish to integrate this patch, please provide the proper attribution :) --Zem From: Ziemowit Laski Sent: Monday, 6 February 2017 12:38 To: 'jan.nijtm...@gmail.com'; 'sqlite-users@mailinglists.sqlite.org' Subject: SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: [sqlite] Patch Etiquette) Hi Jan, Thank you for the link. I'll take a look. --Zem [P.S. It appears I cannot reply to the whole list directly from the https://www.mail-archive.com/sqlite-users webpage. Is this because I'm not (yet) a members of the list?] ___ sqlite-users mailing list sqlite-users@mailinglists.sqlite.org http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
[sqlite] SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)
Hi Jan, Thank you for the link. I'll take a look. --Zem [P.S. It appears I cannot reply to the whole list directly from the https://www.mail-archive.com/sqlite-users webpage. Is this because I'm not (yet) a members of the list?] ___ sqlite-users mailing list sqlite-users@mailinglists.sqlite.org http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users