Quantcast

SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)

Ziemowit Laski
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
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)

Ziemowit Laski
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: '[hidden email]'; '[hidden email]'
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
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)

Ziemowit Laski
In reply to this post by Ziemowit Laski
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.000000000 -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.000000000 -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 -C5 sqlitesrc2/sqlite-src-3160200/Makefile.msc sqlitesrc3/sqlite-src-3160200/Makefile.msc
*** sqlitesrc2/sqlite-src-3160200/Makefile.msc  2017-01-06 08:52:10.000000000 -0800
--- sqlitesrc3/sqlite-src-3160200/Makefile.msc  2017-02-05 20:00:47.125852800 -0800
***************
*** 828,837 ****
--- 828,843 ----
  # specific Tcl shell to use.
  #
  !IFNDEF TCLSH_CMD
  TCLSH_CMD = tclsh
  !ENDIF
+
+ # The command to use for diff, analogous to tclsh.
+ !IFNDEF DIFF_CMD
+ DIFF_CMD = diff
+ !ENDIF
+
  # <</mark>>

  # Compiler options needed for programs that use the readline() library.
  #
  !IFNDEF READLINE_FLAGS
***************
*** 1910,1926 ****

  opcodes.h:    parse.h $(TOP)\src\vdbe.c $(TOP)\tool\mkopcodeh.tcl
        type 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.exe $(TOP)\tool\addopcodes.tcl
        del /Q parse.y parse.h parse.h.temp 2>NUL
        copy $(TOP)\src\parse.y .
!       .\lemon.exe $(REQ_FEATURE_FLAGS) $(OPT_FEATURE_FLAGS) $(EXT_FEATURE_FLAGS) $(OPTS) parse.y
        move parse.h parse.h.temp
        $(TCLSH_CMD) $(TOP)\tool\addopcodes.tcl parse.h.temp > parse.h

  $(SQLITE3H):  $(TOP)\src\sqlite.h.in $(TOP)\manifest.uuid $(TOP)\VERSION
        $(TCLSH_CMD) $(TOP)\tool\mksqlite3h.tcl $(TOP:\=/) > $(SQLITE3H) $(MKSQLITE3H_ARGS)
--- 1916,1949 ----

  opcodes.h:    parse.h $(TOP)\src\vdbe.c $(TOP)\tool\mkopcodeh.tcl
        type 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.exe $(TOP)\tool\addopcodes.tcl
        del /Q parse.y parse.h parse.h.temp 2>NUL
        copy $(TOP)\src\parse.y .
!       .\lemon.exe $(REQ_FEATURE_FLAGS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) \
!               $(OPT_FEATURE_FLAGS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) \
!               $(EXT_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.exe $(REQ_FEATURE_FLAGS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) \
!               $(OPT_FEATURE_FLAGS:SQLITE_ENABLE_UPDATE_DELETE_LIMIT=_UNUSED) \
!               $(EXT_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_CMD) -d -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT parse.c.1.temp parse.c.2.temp >parse.c
        move parse.h parse.h.temp
        $(TCLSH_CMD) $(TOP)\tool\addopcodes.tcl parse.h.temp > parse.h

  $(SQLITE3H):  $(TOP)\src\sqlite.h.in $(TOP)\manifest.uuid $(TOP)\VERSION
        $(TCLSH_CMD) $(TOP)\tool\mksqlite3h.tcl $(TOP:\=/) > $(SQLITE3H) $(MKSQLITE3H_ARGS)
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)

Jan Nijtmans
In reply to this post by Ziemowit Laski
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
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: SQLITE_ENABLE_UPDATE_DELETE_LIMIT (Was: Patch Etiquette)

Ziemowit Laski
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:[hidden email]]
Sent: Wednesday, 8 February 2017 2:58
To: Ziemowit Laski
Cc: [hidden email]
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
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Loading...