[sqlite] SQLITE_ENABLE_UPDATE_DELETE_LIMIT - my "final" patch
Hello again, So after discussions with Jan and further contemplation, I concluded that the only way to get a hold on the '#line ... ' issues in the generated parse.c is to write a dedicated tool to do it. The patch herein contains such a tool, named 'lineclean'. The tool detects sequences of the form #if(n)def #line ... "parse.c" #else #line ... "parse.c" #endif and replaces them with a single line #line ... "parse.c" Also, lineclean fixes up all '#line ... "parse.c"' commands so that they actually refer to the next line in the file. (This was completely broken as a result of running the diff). Anyhoo, since I still haven't heard from any of the maintainers, I'll leave it at that. Should you decide to apply the patch (with attribution, of course :-)) and run into problems, please e-mail me directly (zlaski _at_ ziemas _dot_ net) since I'm not a member of sqlite-users. Thank you, --Zem ___ 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)
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, &I); 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, &I); 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
Re: [sqlite] Patch Etiquette
Hi Jens, Yes, I did notice the commit that mentions my name, and am glad for the attribution. What I was getting at, though, is that such attributions should be part of SQLite policy and should happen automatically and every time. To your question, I don't particularly care if I have write privileges to the repo. In fact, I don't think it would be a good idea because (1) I'm a relative newbie to SQLite and could easily break things and (2) I'm not and will not be a regular contributor. I DO care about being acknowledged as the author of the patch, however. On the e-mail address question, I tend to think that they should be required as well. Any patch that is committed may cause integration problems down the line. SQLite has numerous extensions which can be baked into it, and it is impossible for any single contributor to perform regression testing on all these permutations. Ergo, it may be necessary to contact the author of the patch for them to try to rework it and/or explain how the patch's functionality is supposed to interoperate with the new feature/change which caused the breakage. SQLite is a relatively small project, so we can probably get away with not publishing e-mail addresses, since the 3 maintainers probably know who submitted what and can contact them if needed. For larger projects (e.g., GCC), having e-mail addresses of contributors known to everyone is absolutely essential. Thanks, --Zem ___ 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
[sqlite] Patch Etiquette
Hello SQLite maintainers, I'm glad that my patch below has been incorporated (with some changes) into future releases: [50e60cb4]<http://www.sqlite.org/src/info/50e60cb44fd3687d> Modify the ICU extension to use a static initializer, as VC++ complains about a dynamic initialization. Maybe the dynamic structure initialization is a GCC extension. (user: drh<http://www.sqlite.org/src/timeline?u=drh&c=2017-01-26+00%3A58%3A27&nd&n=200>, tags: trunk<http://www.sqlite.org/src/timeline?r=trunk&nd&c=2017-01-26+00%3A58%3A27&n=200>) I think that SQLite is a great product, and I'm happy to be able to contribute to it. HOWEVER, one thing bothers me: You did not acknowledge my authorship of it. AFAICT, you NEVER seem to acknowledge third party contributions. Clearly, I'm not user 'drh'. Like with other open-source projects, I would expect my name and e-mail to appear in the commit message (and the changelog, if you had one), and definitely in the release notes. This is the polite and professional thing to do, and may even encourage others to contribute to the project. I have another patch in the works which will make SQLITE_ENABLE_UPDATE_DELETE_LIMIT directly accessible from the amalgamation, but am reluctant to submit it. Thank you, --Zem From: Ziemowit Laski Sent: Wednesday, 25 January 2017 12:36 To: sqlite-users@mailinglists.sqlite.org Subject: BUG: Illegal initialization in icu.c : sqlite3IcuInit Visual C++ correctly catches this. The fragment struct IcuScalar { const char *zName;/* Function name */ int nArg; /* Number of arguments */ int enc; /* Optimal text encoding */ void *pContext; /* sqlite3_user_data() context */ void (*xFunc)(sqlite3_context*,int,sqlite3_value**); } scalars[] = { {"regexp", 2, SQLITE_ANY|SQLITE_DETERMINISTIC, 0, icuRegexpFunc}, {"lower", 1, SQLITE_UTF16|SQLITE_DETERMINISTIC,0, icuCaseFunc16}, {"lower", 2, SQLITE_UTF16|SQLITE_DETERMINISTIC,0, icuCaseFunc16}, {"upper", 1, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16}, {"upper", 2, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16}, {"lower", 1, SQLITE_UTF8|SQLITE_DETERMINISTIC, 0, icuCaseFunc16}, {"lower", 2, SQLITE_UTF8|SQLITE_DETERMINISTIC, 0, icuCaseFunc16}, {"upper", 1, SQLITE_UTF8|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16}, {"upper", 2, SQLITE_UTF8|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16}, {"like", 2, SQLITE_UTF8|SQLITE_DETERMINISTIC, 0, icuLikeFunc}, {"like", 3, SQLITE_UTF8|SQLITE_DETERMINISTIC, 0, icuLikeFunc}, {"icu_load_collation", 2, SQLITE_UTF8, (void*)db, icuLoadCollation}, }; int rc = SQLITE_OK; int i; should read struct IcuScalar { const char *zName;/* Function name */ int nArg; /* Number of arguments */ int enc; /* Optimal text encoding */ void *pContext; /* sqlite3_user_data() context */ void(*xFunc)(sqlite3_context*, int, sqlite3_value**); } scalars[] = { { "regexp", 2, SQLITE_ANY | SQLITE_DETERMINISTIC, 0, icuRegexpFunc }, { "lower", 1, SQLITE_UTF16 | SQLITE_DETERMINISTIC,0, icuCaseFunc16 }, { "lower", 2, SQLITE_UTF16 | SQLITE_DETERMINISTIC,0, icuCaseFunc16 }, { "upper", 1, SQLITE_UTF16 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 }, { "upper", 2, SQLITE_UTF16 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 }, { "lower", 1, SQLITE_UTF8 | SQLITE_DETERMINISTIC, 0, icuCaseFunc16 }, { "lower", 2, SQLITE_UTF8 | SQLITE_DETERMINISTIC, 0, icuCaseFunc16 }, { "upper", 1, SQLITE_UTF8 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 }, { "upper", 2, SQLITE_UTF8 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 }, { "like", 2, SQLITE_UTF8 | SQLITE_DETERMINISTIC, 0, icuLikeFunc }, { "like", 3, SQLITE_UTF8 | SQLITE_DETERMINISTIC, 0, icuLikeFunc }, { "icu_load_collation", 2, SQLITE_UTF8, 0, icuLoadCollation } }; int rc = SQLITE_OK; int i; scalars[11].pContext = (void*)db; Thank you, --Zem ___ sqlite-users mailing list sqlite-users@mailinglists.sqlite.org http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
[sqlite] BUG: Illegal initialization in icu.c : sqlite3IcuInit
Visual C++ correctly catches this. The fragment struct IcuScalar { const char *zName;/* Function name */ int nArg; /* Number of arguments */ int enc; /* Optimal text encoding */ void *pContext; /* sqlite3_user_data() context */ void (*xFunc)(sqlite3_context*,int,sqlite3_value**); } scalars[] = { {"regexp", 2, SQLITE_ANY|SQLITE_DETERMINISTIC, 0, icuRegexpFunc}, {"lower", 1, SQLITE_UTF16|SQLITE_DETERMINISTIC,0, icuCaseFunc16}, {"lower", 2, SQLITE_UTF16|SQLITE_DETERMINISTIC,0, icuCaseFunc16}, {"upper", 1, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16}, {"upper", 2, SQLITE_UTF16|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16}, {"lower", 1, SQLITE_UTF8|SQLITE_DETERMINISTIC, 0, icuCaseFunc16}, {"lower", 2, SQLITE_UTF8|SQLITE_DETERMINISTIC, 0, icuCaseFunc16}, {"upper", 1, SQLITE_UTF8|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16}, {"upper", 2, SQLITE_UTF8|SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16}, {"like", 2, SQLITE_UTF8|SQLITE_DETERMINISTIC, 0, icuLikeFunc}, {"like", 3, SQLITE_UTF8|SQLITE_DETERMINISTIC, 0, icuLikeFunc}, {"icu_load_collation", 2, SQLITE_UTF8, (void*)db, icuLoadCollation}, }; int rc = SQLITE_OK; int i; should read struct IcuScalar { const char *zName;/* Function name */ int nArg; /* Number of arguments */ int enc; /* Optimal text encoding */ void *pContext; /* sqlite3_user_data() context */ void(*xFunc)(sqlite3_context*, int, sqlite3_value**); } scalars[] = { { "regexp", 2, SQLITE_ANY | SQLITE_DETERMINISTIC, 0, icuRegexpFunc }, { "lower", 1, SQLITE_UTF16 | SQLITE_DETERMINISTIC,0, icuCaseFunc16 }, { "lower", 2, SQLITE_UTF16 | SQLITE_DETERMINISTIC,0, icuCaseFunc16 }, { "upper", 1, SQLITE_UTF16 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 }, { "upper", 2, SQLITE_UTF16 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 }, { "lower", 1, SQLITE_UTF8 | SQLITE_DETERMINISTIC, 0, icuCaseFunc16 }, { "lower", 2, SQLITE_UTF8 | SQLITE_DETERMINISTIC, 0, icuCaseFunc16 }, { "upper", 1, SQLITE_UTF8 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 }, { "upper", 2, SQLITE_UTF8 | SQLITE_DETERMINISTIC, (void*)1, icuCaseFunc16 }, { "like", 2, SQLITE_UTF8 | SQLITE_DETERMINISTIC, 0, icuLikeFunc }, { "like", 3, SQLITE_UTF8 | SQLITE_DETERMINISTIC, 0, icuLikeFunc }, { "icu_load_collation", 2, SQLITE_UTF8, 0, icuLoadCollation } }; int rc = SQLITE_OK; int i; scalars[11].pContext = (void*)db; Thank you, --Zem ___ sqlite-users mailing list sqlite-users@mailinglists.sqlite.org http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users