unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#70145: [PATCH] Add sqlite-execute-batch command
@ 2024-04-02 15:03 Javier Olaechea
  2024-04-02 16:21 ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Javier Olaechea @ 2024-04-02 15:03 UTC (permalink / raw)
  To: 70145


[-- Attachment #1.1: Type: text/plain, Size: 1034 bytes --]

Hi, while writing tests for an Emacs package I found myself needing to
execute multiple statements against an in-memory database, to initialize
the schema. Currently there is no easy way to do so as sqlite-execute
only executes the first command and ignores the rest. The reason most
likely being that accepting arguments for multiple statements and
properly preparing would be a tricky task. So instead I'm adding the
functionality as a new function that takes no arguments.


In GNU Emacs 29.1 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
version 1.18.0, Xaw3d scroll bars)
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Ubuntu 20.04.6 LTS

Configured using:
 'configure
 --prefix=/nix/store/0g4xxdsn4xp9qhgc4cylbksqpwsn51vc-emacs-29.1
 --disable-build-details --with-modules --with-x-toolkit=lucid
 --with-xft --with-cairo --with-native-compilation --with-tree-sitter
 --with-xinput2'

-- 
"I object to doing things that computers can do." — Olin Shivers

[-- Attachment #1.2: Type: text/html, Size: 1268 bytes --]

[-- Attachment #2: 0001-Add-sqlite-execute-batch-command.patch --]
[-- Type: text/x-patch, Size: 2202 bytes --]

From 454b23cb31332fbd5b5d2c5117394c578581b72b Mon Sep 17 00:00:00 2001
From: Javier Olaechea <pirata@gmail.com>
Date: Sun, 31 Mar 2024 23:07:10 -0500
Subject: [PATCH] Add sqlite-execute-batch command

This command is similar to sqlite-execute except that it executes
multiple statements in exchange for not accepting any arguments.

* doc/lispref/text.texi (Database): Document it.
* src/sqlite.c (Fsqlite_execute_batch): Add sqlite_execute_batch
command. It is similar to sqlite-execute but it executes all the
statements in the query. Unlike sqlite-execute the command doesn't take
any arguments to pass down to the statements.
---
 doc/lispref/text.texi |  6 ++++++
 src/sqlite.c          | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 90e2c6ce882..cad6df52e55 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -5404,6 +5404,12 @@ Database
 
 @end defun
 
+@defun sqlite-execute-batch db statements
+Execute the @acronym{SQL} @var{statements}. This might be useful when we
+want to execute multiple @acronym{DDL} statements.
+
+@end defun
+
 @defun sqlite-select db query &optional values return-type
 Select some data from @var{db} and return them.  For instance:
 
diff --git a/src/sqlite.c b/src/sqlite.c
index 261080da673..043801459d2 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -646,6 +646,15 @@ sqlite_exec (sqlite3 *sdb, const char *query)
   return Qt;
 }
 
+DEFUN ("sqlite-execute-batch", Fsqlite_execute_batch, Ssqlite_execute_batch, 2, 2, 0,
+       doc: /* Execute multiple SQL statements.  */)
+  (Lisp_Object db, Lisp_Object query)
+{
+  check_sqlite (db, false);
+  CHECK_STRING (query);
+  return sqlite_exec (XSQLITE (db)->db, SSDATA (query));
+}
+
 DEFUN ("sqlite-transaction", Fsqlite_transaction, Ssqlite_transaction, 1, 1, 0,
        doc: /* Start a transaction in DB.  */)
   (Lisp_Object db)
@@ -866,6 +875,7 @@ syms_of_sqlite (void)
   defsubr (&Ssqlite_close);
   defsubr (&Ssqlite_execute);
   defsubr (&Ssqlite_select);
+  defsubr (&Ssqlite_execute_batch);
   defsubr (&Ssqlite_transaction);
   defsubr (&Ssqlite_commit);
   defsubr (&Ssqlite_rollback);
-- 
2.29.2.154.g7f7ebe054a


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#70145: [PATCH] Add sqlite-execute-batch command
  2024-04-02 15:03 bug#70145: [PATCH] Add sqlite-execute-batch command Javier Olaechea
@ 2024-04-02 16:21 ` Eli Zaretskii
  2024-04-02 18:52   ` Javier Olaechea
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-04-02 16:21 UTC (permalink / raw)
  To: Javier Olaechea; +Cc: 70145

> From: Javier Olaechea <pirata@gmail.com>
> Date: Tue, 2 Apr 2024 10:03:30 -0500
> 
> Hi, while writing tests for an Emacs package I found myself needing to
> execute multiple statements against an in-memory database, to initialize
> the schema. Currently there is no easy way to do so as sqlite-execute
> only executes the first command and ignores the rest. The reason most
> likely being that accepting arguments for multiple statements and
> properly preparing would be a tricky task. So instead I'm adding the
> functionality as a new function that takes no arguments.

Thanks.

> +DEFUN ("sqlite-execute-batch", Fsqlite_execute_batch, Ssqlite_execute_batch, 2, 2, 0,
> +       doc: /* Execute multiple SQL statements.  */)
> +  (Lisp_Object db, Lisp_Object query)
> +{
> +  check_sqlite (db, false);
> +  CHECK_STRING (query);
> +  return sqlite_exec (XSQLITE (db)->db, SSDATA (query));
> +}
> +

This has a subtle bug: it will only work correctly for plain-ASCII
string in QUERY.  If QUERY is allowed to included non-ASCII
characters, you need to encode it using encode_string.

Also, the doc string should mention the function's arguments, and it
should say what kind of Lisp object is QUERY.

> +@defun sqlite-execute-batch db statements
> +Execute the @acronym{SQL} @var{statements}. This might be useful when we
> +want to execute multiple @acronym{DDL} statements.

Same here, and in addition please leave two spaces between sentences,
per our conventions (in the commit log message as well).





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#70145: [PATCH] Add sqlite-execute-batch command
  2024-04-02 16:21 ` Eli Zaretskii
@ 2024-04-02 18:52   ` Javier Olaechea
  2024-04-02 18:56     ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Javier Olaechea @ 2024-04-02 18:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70145


[-- Attachment #1.1: Type: text/plain, Size: 293 bytes --]

Thanks, I've updated the patch according to the feedback.

Btw I have a basic test for the functionality which I used to validate the
code before submitting the patch. Should I include it in
test/sqlite-tests.el?

-- 
"I object to doing things that computers can do." — Olin Shivers

[-- Attachment #1.2: Type: text/html, Size: 455 bytes --]

[-- Attachment #2: 0001-Add-sqlite-execute-batch-command.patch --]
[-- Type: text/x-patch, Size: 2407 bytes --]

From 6c984799a3936300bbde62e24cce21de19fa3b3f Mon Sep 17 00:00:00 2001
From: Javier Olaechea <pirata@gmail.com>
Date: Sun, 31 Mar 2024 23:07:10 -0500
Subject: [PATCH] Add sqlite-execute-batch command

This command is similar to sqlite-execute except that it executes
multiple statements in exchange for not accepting any arguments.

* doc/lispref/text.texi (Database): Document it.
* src/sqlite.c (Fsqlite_execute_batch): Add sqlite_execute_batch
command.  It is similar to sqlite-execute but it executes all the
statements in the query.  Unlike sqlite-execute the command doesn't take
any arguments to pass down to the statements.
---
 doc/lispref/text.texi |  8 ++++++++
 src/sqlite.c          | 12 ++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 90e2c6ce882..f1f8f813981 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -5404,6 +5404,14 @@ Database
 
 @end defun
 
+@defun sqlite-execute-batch db statements
+Execute the @acronym{SQL} @var{statements}.  @var{statements} is a
+string containing 0 or more @acronym{SQL} statements.  This command
+might be useful when we want to execute multiple @acronym{DDL}
+statements.
+
+@end defun
+
 @defun sqlite-select db query &optional values return-type
 Select some data from @var{db} and return them.  For instance:
 
diff --git a/src/sqlite.c b/src/sqlite.c
index 261080da673..c606fa5f831 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -646,6 +646,17 @@ sqlite_exec (sqlite3 *sdb, const char *query)
   return Qt;
 }
 
+DEFUN ("sqlite-execute-batch", Fsqlite_execute_batch, Ssqlite_execute_batch, 2, 2, 0,
+       doc: /* Execute multiple SQL statements in DB.
+Query is a string containing 0 or more SQL statements.  */)
+  (Lisp_Object db, Lisp_Object query)
+{
+  check_sqlite (db, false);
+  CHECK_STRING (query);
+  Lisp_Object encoded = encode_string(query);
+  return sqlite_exec (XSQLITE (db)->db, SSDATA (encoded));
+}
+
 DEFUN ("sqlite-transaction", Fsqlite_transaction, Ssqlite_transaction, 1, 1, 0,
        doc: /* Start a transaction in DB.  */)
   (Lisp_Object db)
@@ -866,6 +877,7 @@ syms_of_sqlite (void)
   defsubr (&Ssqlite_close);
   defsubr (&Ssqlite_execute);
   defsubr (&Ssqlite_select);
+  defsubr (&Ssqlite_execute_batch);
   defsubr (&Ssqlite_transaction);
   defsubr (&Ssqlite_commit);
   defsubr (&Ssqlite_rollback);
-- 
2.29.2.154.g7f7ebe054a


[-- Attachment #3: smoke.el --]
[-- Type: text/x-emacs-lisp, Size: 742 bytes --]

;; -*- lexical-binding: t -*-

(require 'ert)

(ert-deftest with-sqlite-execute-batch-test ()
  (let ((db (sqlite-open nil))
        (query (with-temp-buffer
                 (insert "-- -*- sql-product: sqlite -*-

-- I 💘 emojis

CREATE TABLE settings (
  name TEXT NOT NULL,
  value TEXT,
  section TEXT NOT NULL,
  PRIMARY KEY (section, name)
);

CREATE TABLE tags (
  name TEXT PRIMARY KEY NOT NULL
);

-- CREATE TABLE todo_states (id INTEGER PRIMARY KEY, name TEXT NOT NULL);
")
                 (buffer-string))))
    (sqlite-execute-batch db query)
    (should (equal '(("settings") ("tags"))
                   (sqlite-select db "select name from sqlite_master where type = 'table' and name not like 'sqlite_%' order by name")))))

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#70145: [PATCH] Add sqlite-execute-batch command
  2024-04-02 18:52   ` Javier Olaechea
@ 2024-04-02 18:56     ` Eli Zaretskii
  2024-04-02 19:10       ` Javier Olaechea
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-04-02 18:56 UTC (permalink / raw)
  To: Javier Olaechea; +Cc: 70145

> From: Javier Olaechea <pirata@gmail.com>
> Date: Tue, 2 Apr 2024 13:52:12 -0500
> Cc: 70145@debbugs.gnu.org
> 
> Thanks, I've updated the patch according to the feedback. 

Thanks, will review soon.

> Btw I have a basic test for the functionality which I used to validate the code before submitting the patch.
> Should I include it in test/sqlite-tests.el?

Yes, more tests are always welcome.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#70145: [PATCH] Add sqlite-execute-batch command
  2024-04-02 18:56     ` Eli Zaretskii
@ 2024-04-02 19:10       ` Javier Olaechea
  2024-04-03  1:56         ` J.P.
  0 siblings, 1 reply; 10+ messages in thread
From: Javier Olaechea @ 2024-04-02 19:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70145


[-- Attachment #1.1: Type: text/plain, Size: 628 bytes --]

Ok, updated the patch to include the test

On Tue, Apr 2, 2024 at 1:56 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Javier Olaechea <pirata@gmail.com>
> > Date: Tue, 2 Apr 2024 13:52:12 -0500
> > Cc: 70145@debbugs.gnu.org
> >
> > Thanks, I've updated the patch according to the feedback.
>
> Thanks, will review soon.
>
> > Btw I have a basic test for the functionality which I used to validate
> the code before submitting the patch.
> > Should I include it in test/sqlite-tests.el?
>
> Yes, more tests are always welcome.
>


-- 
"I object to doing things that computers can do." — Olin Shivers

[-- Attachment #1.2: Type: text/html, Size: 1182 bytes --]

[-- Attachment #2: 0001-Add-sqlite-execute-batch-command.patch --]
[-- Type: text/x-patch, Size: 3630 bytes --]

From b0cbe05c7da8abb49b45a799c1b066e788cf1c6d Mon Sep 17 00:00:00 2001
From: Javier Olaechea <pirata@gmail.com>
Date: Sun, 31 Mar 2024 23:07:10 -0500
Subject: [PATCH] Add sqlite-execute-batch command

This command is similar to sqlite-execute except that it executes
multiple statements in exchange for not accepting any arguments.

* doc/lispref/text.texi (Database): Document it.
* src/sqlite.c (Fsqlite_execute_batch): Add sqlite_execute_batch
command.  It is similar to sqlite-execute but it executes all the
statements in the query.  Unlike sqlite-execute the command doesn't take
any arguments to pass down to the statements.
* test/lisp/sqlite-tests.el (with-sqlite-execute-batch-test): Test it.
---
 doc/lispref/text.texi     |  8 ++++++++
 src/sqlite.c              | 12 ++++++++++++
 test/lisp/sqlite-tests.el | 25 +++++++++++++++++++++++++
 3 files changed, 45 insertions(+)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 90e2c6ce882..f1f8f813981 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -5404,6 +5404,14 @@ Database
 
 @end defun
 
+@defun sqlite-execute-batch db statements
+Execute the @acronym{SQL} @var{statements}.  @var{statements} is a
+string containing 0 or more @acronym{SQL} statements.  This command
+might be useful when we want to execute multiple @acronym{DDL}
+statements.
+
+@end defun
+
 @defun sqlite-select db query &optional values return-type
 Select some data from @var{db} and return them.  For instance:
 
diff --git a/src/sqlite.c b/src/sqlite.c
index 261080da673..c606fa5f831 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -646,6 +646,17 @@ sqlite_exec (sqlite3 *sdb, const char *query)
   return Qt;
 }
 
+DEFUN ("sqlite-execute-batch", Fsqlite_execute_batch, Ssqlite_execute_batch, 2, 2, 0,
+       doc: /* Execute multiple SQL statements in DB.
+Query is a string containing 0 or more SQL statements.  */)
+  (Lisp_Object db, Lisp_Object query)
+{
+  check_sqlite (db, false);
+  CHECK_STRING (query);
+  Lisp_Object encoded = encode_string(query);
+  return sqlite_exec (XSQLITE (db)->db, SSDATA (encoded));
+}
+
 DEFUN ("sqlite-transaction", Fsqlite_transaction, Ssqlite_transaction, 1, 1, 0,
        doc: /* Start a transaction in DB.  */)
   (Lisp_Object db)
@@ -866,6 +877,7 @@ syms_of_sqlite (void)
   defsubr (&Ssqlite_close);
   defsubr (&Ssqlite_execute);
   defsubr (&Ssqlite_select);
+  defsubr (&Ssqlite_execute_batch);
   defsubr (&Ssqlite_transaction);
   defsubr (&Ssqlite_commit);
   defsubr (&Ssqlite_rollback);
diff --git a/test/lisp/sqlite-tests.el b/test/lisp/sqlite-tests.el
index d4892a27efc..7053026eb82 100644
--- a/test/lisp/sqlite-tests.el
+++ b/test/lisp/sqlite-tests.el
@@ -48,4 +48,29 @@ with-sqlite-transaction/rollback
     ;; First insertion (a=1) rolled back.
     (should-not (sqlite-select db "select * from test"))))
 
+(ert-deftest with-sqlite-execute-batch-test ()
+  (let ((db (sqlite-open nil))
+        (query (with-temp-buffer
+                 (insert "-- -*- sql-product: sqlite -*-
+
+-- I 💘 emojis
+
+CREATE TABLE settings (
+  name TEXT NOT NULL,
+  value TEXT,
+  section TEXT NOT NULL,
+  PRIMARY KEY (section, name)
+);
+
+CREATE TABLE tags📎 (
+  name TEXT PRIMARY KEY NOT NULL
+);
+
+-- CREATE TABLE todo_states (id INTEGER PRIMARY KEY, name TEXT NOT NULL);
+")
+                 (buffer-string))))
+    (sqlite-execute-batch db query)
+    (should (equal '(("settings") ("tags📎"))
+                   (sqlite-select db "select name from sqlite_master where type = 'table' and name not like 'sqlite_%' order by name")))))
+
 ;;; sqlite-tests.el ends here
-- 
2.29.2.154.g7f7ebe054a


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#70145: [PATCH] Add sqlite-execute-batch command
  2024-04-02 19:10       ` Javier Olaechea
@ 2024-04-03  1:56         ` J.P.
  2024-04-03  2:42           ` Javier Olaechea
  0 siblings, 1 reply; 10+ messages in thread
From: J.P. @ 2024-04-03  1:56 UTC (permalink / raw)
  To: Javier Olaechea; +Cc: 70145, Eli Zaretskii

Hi Javier,

Thanks for working on this. I think Emacs applications will find it
handy for things like initializing a new database. Just a couple things
I spotted in passing. (I don't really know anything, though, so feel
free to ignore.)

Javier Olaechea <pirata@gmail.com> writes:

> * test/lisp/sqlite-tests.el (with-sqlite-execute-batch-test): Test it.
         ~~~~

I think there's also a test/src/sqlite-tests.el file, which seems to
contain tests for those primitives defined in src/sqlite.c. Perhaps this
test should go there instead?

> diff --git a/test/lisp/sqlite-tests.el b/test/lisp/sqlite-tests.el
> index d4892a27efc..7053026eb82 100644
> --- a/test/lisp/sqlite-tests.el
> +++ b/test/lisp/sqlite-tests.el
> @@ -48,4 +48,29 @@ with-sqlite-transaction/rollback
>      ;; First insertion (a=1) rolled back.
>      (should-not (sqlite-select db "select * from test"))))
>  
> +(ert-deftest with-sqlite-execute-batch-test ()

Maybe I'm mistaken, but I don't think you need the "with-" part (or the
"-test" part), no?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#70145: [PATCH] Add sqlite-execute-batch command
  2024-04-03  1:56         ` J.P.
@ 2024-04-03  2:42           ` Javier Olaechea
  2024-04-13  8:03             ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Javier Olaechea @ 2024-04-03  2:42 UTC (permalink / raw)
  To: J.P.; +Cc: 70145, Eli Zaretskii


[-- Attachment #1.1: Type: text/plain, Size: 1644 bytes --]

> Thanks for working on this. I think Emacs applications will find it
> handy for things like initializing a new database. Just a couple things
> I spotted in passing. (I don't really know anything, though, so feel
> free to ignore.)
>

Feedback is always welcome. It's not like I know anything either ^_^'.


> I think there's also a test/src/sqlite-tests.el file, which seems to
> contain tests for those primitives defined in src/sqlite.c. Perhaps this
> test should go there instead?
>
>
Thanks for pointing that out, I hadn't seen that file. It does seem like a
better home for the test.


> > diff --git a/test/lisp/sqlite-tests.el b/test/lisp/sqlite-tests.el
> > index d4892a27efc..7053026eb82 100644
> > --- a/test/lisp/sqlite-tests.el
> > +++ b/test/lisp/sqlite-tests.el
> > @@ -48,4 +48,29 @@ with-sqlite-transaction/rollback
> >      ;; First insertion (a=1) rolled back.
> >      (should-not (sqlite-select db "select * from test"))))
> >
> > +(ert-deftest with-sqlite-execute-batch-test ()
>
> Maybe I'm mistaken, but I don't think you need the "with-" part (or the
> "-test" part), no?
>

I did find it contrary to the foo-test- prefix I've seen in the elisp code
in the wild, but on matters of naming conventions I try to follow the
motto: "When in Rome, do as Romans do". So I named the test according to
the other tests in the file. Now that I placed it under
test/src/sqlite-tests.el I've changed the name to align with the names of
the test in that file.

I've attached a new patch incorporating your feedback

-- 
"I object to doing things that computers can do." — Olin Shivers

[-- Attachment #1.2: Type: text/html, Size: 2510 bytes --]

[-- Attachment #2: 0001-Add-sqlite-execute-batch-command.patch --]
[-- Type: text/x-patch, Size: 3622 bytes --]

From 59c044816fbd4283a134efa5454a38a985980b2b Mon Sep 17 00:00:00 2001
From: Javier Olaechea <pirata@gmail.com>
Date: Sun, 31 Mar 2024 23:07:10 -0500
Subject: [PATCH] Add sqlite-execute-batch command

This command is similar to sqlite-execute except that it executes
multiple statements in exchange for not accepting any arguments.

* doc/lispref/text.texi (Database): Document it.
* src/sqlite.c (Fsqlite_execute_batch): Add sqlite_execute_batch
command.  It is similar to sqlite-execute but it executes all the
statements in the query.  Unlike sqlite-execute the command doesn't take
any arguments to pass down to the statements.
* test/src/sqlite-tests.el (sqlite-multiple-statements): Add smoke test
for sqlite-execute-batch.
---
 doc/lispref/text.texi    |  8 ++++++++
 src/sqlite.c             | 12 ++++++++++++
 test/src/sqlite-tests.el | 26 ++++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 90e2c6ce882..f1f8f813981 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -5404,6 +5404,14 @@ Database
 
 @end defun
 
+@defun sqlite-execute-batch db statements
+Execute the @acronym{SQL} @var{statements}.  @var{statements} is a
+string containing 0 or more @acronym{SQL} statements.  This command
+might be useful when we want to execute multiple @acronym{DDL}
+statements.
+
+@end defun
+
 @defun sqlite-select db query &optional values return-type
 Select some data from @var{db} and return them.  For instance:
 
diff --git a/src/sqlite.c b/src/sqlite.c
index 261080da673..c606fa5f831 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -646,6 +646,17 @@ sqlite_exec (sqlite3 *sdb, const char *query)
   return Qt;
 }
 
+DEFUN ("sqlite-execute-batch", Fsqlite_execute_batch, Ssqlite_execute_batch, 2, 2, 0,
+       doc: /* Execute multiple SQL statements in DB.
+Query is a string containing 0 or more SQL statements.  */)
+  (Lisp_Object db, Lisp_Object query)
+{
+  check_sqlite (db, false);
+  CHECK_STRING (query);
+  Lisp_Object encoded = encode_string(query);
+  return sqlite_exec (XSQLITE (db)->db, SSDATA (encoded));
+}
+
 DEFUN ("sqlite-transaction", Fsqlite_transaction, Ssqlite_transaction, 1, 1, 0,
        doc: /* Start a transaction in DB.  */)
   (Lisp_Object db)
@@ -866,6 +877,7 @@ syms_of_sqlite (void)
   defsubr (&Ssqlite_close);
   defsubr (&Ssqlite_execute);
   defsubr (&Ssqlite_select);
+  defsubr (&Ssqlite_execute_batch);
   defsubr (&Ssqlite_transaction);
   defsubr (&Ssqlite_commit);
   defsubr (&Ssqlite_rollback);
diff --git a/test/src/sqlite-tests.el b/test/src/sqlite-tests.el
index a10dca9a0c9..e87a5fc77b1 100644
--- a/test/src/sqlite-tests.el
+++ b/test/src/sqlite-tests.el
@@ -261,4 +261,30 @@ sqlite-returning
 		        '("Joe" "Doe"))
         '((1 "Joe")))))))
 
+(ert-deftest sqlite-multiple-statements ()
+  (skip-unless (sqlite-available-p))
+  (let ((db (sqlite-open nil))
+        (query (with-temp-buffer
+                 (insert "-- -*- sql-product: sqlite -*-
+
+-- I 💘 emojis
+
+CREATE TABLE settings (
+  name TEXT NOT NULL,
+  value TEXT,
+  section TEXT NOT NULL,
+  PRIMARY KEY (section, name)
+);
+
+CREATE TABLE tags📎 (
+  name TEXT PRIMARY KEY NOT NULL
+);
+
+-- CREATE TABLE todo_states (id INTEGER PRIMARY KEY, name TEXT NOT NULL);
+")
+                 (buffer-string))))
+    (sqlite-execute-batch db query)
+    (should (equal '(("settings") ("tags📎"))
+                   (sqlite-select db "select name from sqlite_master where type = 'table' and name not like 'sqlite_%' order by name")))))
+
 ;;; sqlite-tests.el ends here
-- 
2.29.2.154.g7f7ebe054a


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#70145: [PATCH] Add sqlite-execute-batch command
  2024-04-03  2:42           ` Javier Olaechea
@ 2024-04-13  8:03             ` Eli Zaretskii
  2024-04-14  3:32               ` Javier Olaechea
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-04-13  8:03 UTC (permalink / raw)
  To: Javier Olaechea; +Cc: 70145, jp

> From: Javier Olaechea <pirata@gmail.com>
> Date: Tue, 2 Apr 2024 21:42:06 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, 70145@debbugs.gnu.org
> 
>  Thanks for working on this. I think Emacs applications will find it
>  handy for things like initializing a new database. Just a couple things
>  I spotted in passing. (I don't really know anything, though, so feel
>  free to ignore.)
> 
> Feedback is always welcome. It's not like I know anything either ^_^'.
>  
>  I think there's also a test/src/sqlite-tests.el file, which seems to
>  contain tests for those primitives defined in src/sqlite.c. Perhaps this
>  test should go there instead?
> 
> Thanks for pointing that out, I hadn't seen that file. It does seem like a better home for the test.
>  
>  > diff --git a/test/lisp/sqlite-tests.el b/test/lisp/sqlite-tests.el
>  > index d4892a27efc..7053026eb82 100644
>  > --- a/test/lisp/sqlite-tests.el
>  > +++ b/test/lisp/sqlite-tests.el
>  > @@ -48,4 +48,29 @@ with-sqlite-transaction/rollback
>  >      ;; First insertion (a=1) rolled back.
>  >      (should-not (sqlite-select db "select * from test"))))
>  >  
>  > +(ert-deftest with-sqlite-execute-batch-test ()
> 
>  Maybe I'm mistaken, but I don't think you need the "with-" part (or the
>  "-test" part), no?
> 
> I did find it contrary to the foo-test- prefix I've seen in the elisp code in the wild, but on matters of naming
> conventions I try to follow the motto: "When in Rome, do as Romans do". So I named the test according to the
> other tests in the file. Now that I placed it under test/src/sqlite-tests.el I've changed the name to align with the
> names of the test in that file.
> 
> I've attached a new patch incorporating your feedback

Thanks.  This should have a NEWS entry announcing the new command.

Also, to accept this contribution, we'd need a copyright assignment
from you.  Would you like to start the paperwork for that at this
time?  If yes, I will send you the form to fill and the instructions
to go with it.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#70145: [PATCH] Add sqlite-execute-batch command
  2024-04-13  8:03             ` Eli Zaretskii
@ 2024-04-14  3:32               ` Javier Olaechea
  2024-04-14  5:52                 ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Javier Olaechea @ 2024-04-14  3:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70145, jp


[-- Attachment #1.1: Type: text/plain, Size: 469 bytes --]

> Thanks.  This should have a NEWS entry announcing the new command
>

Updated the patch to include the NEWS entry.


> Also, to accept this contribution, we'd need a copyright assignment
> from you.  Would you like to start the paperwork for that at this
> time?  If yes, I will send you the form to fill and the instructions
> to go with it.
>

Yes, please send the form please.

-- 
"I object to doing things that computers can do." — Olin Shivers

[-- Attachment #1.2: Type: text/html, Size: 991 bytes --]

[-- Attachment #2: 0001-Add-sqlite-execute-batch-command.patch --]
[-- Type: text/x-patch, Size: 4247 bytes --]

From 396313cd1caaae71ef215aa9f509c2a4f0e975db Mon Sep 17 00:00:00 2001
From: Javier Olaechea <pirata@gmail.com>
Date: Sun, 31 Mar 2024 23:07:10 -0500
Subject: [PATCH] Add sqlite-execute-batch command

This command is similar to sqlite-execute except that it executes
multiple statements in exchange for not accepting any arguments.

* doc/lispref/text.texi (Database): Document it.
* src/sqlite.c (Fsqlite_execute_batch): Add sqlite_execute_batch
command.  It is similar to sqlite-execute but it executes all the
statements in the query.  Unlike sqlite-execute the command doesn't take
any arguments to pass down to the statements.
* test/src/sqlite-tests.el (sqlite-multiple-statements): Add smoke test
for sqlite-execute-batch.
* etc/NEWS: Mention new command 'sqlite-execute-batch'.
---
 doc/lispref/text.texi    |  8 ++++++++
 etc/NEWS                 |  6 ++++++
 src/sqlite.c             | 12 ++++++++++++
 test/src/sqlite-tests.el | 26 ++++++++++++++++++++++++++
 4 files changed, 52 insertions(+)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 0cd4e2c614e..a1580789d58 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -5414,6 +5414,14 @@ Database
 
 @end defun
 
+@defun sqlite-execute-batch db statements
+Execute the @acronym{SQL} @var{statements}.  @var{statements} is a
+string containing 0 or more @acronym{SQL} statements.  This command
+might be useful when we want to execute multiple @acronym{DDL}
+statements.
+
+@end defun
+
 @defun sqlite-select db query &optional values return-type
 Select some data from @var{db} and return them.  For instance:
 
diff --git a/etc/NEWS b/etc/NEWS
index 7a73815179c..c2a43408da2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -424,6 +424,12 @@ Use 'TAB' in the minibuffer to show or hide the password.  Likewise,
 there is an icon on the mode-line, which toggles the visibility of the
 password when clicking with 'mouse-1'.
 
+** New command 'sqlite-execute-batch'.
+This command lets the user execute multiple SQL commands in one
+command. It is useful when the user wants to evaluate an entire SQL
+file.
+
++++
 \f
 * Editing Changes in Emacs 30.1
 
diff --git a/src/sqlite.c b/src/sqlite.c
index 261080da673..c606fa5f831 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -646,6 +646,17 @@ sqlite_exec (sqlite3 *sdb, const char *query)
   return Qt;
 }
 
+DEFUN ("sqlite-execute-batch", Fsqlite_execute_batch, Ssqlite_execute_batch, 2, 2, 0,
+       doc: /* Execute multiple SQL statements in DB.
+Query is a string containing 0 or more SQL statements.  */)
+  (Lisp_Object db, Lisp_Object query)
+{
+  check_sqlite (db, false);
+  CHECK_STRING (query);
+  Lisp_Object encoded = encode_string(query);
+  return sqlite_exec (XSQLITE (db)->db, SSDATA (encoded));
+}
+
 DEFUN ("sqlite-transaction", Fsqlite_transaction, Ssqlite_transaction, 1, 1, 0,
        doc: /* Start a transaction in DB.  */)
   (Lisp_Object db)
@@ -866,6 +877,7 @@ syms_of_sqlite (void)
   defsubr (&Ssqlite_close);
   defsubr (&Ssqlite_execute);
   defsubr (&Ssqlite_select);
+  defsubr (&Ssqlite_execute_batch);
   defsubr (&Ssqlite_transaction);
   defsubr (&Ssqlite_commit);
   defsubr (&Ssqlite_rollback);
diff --git a/test/src/sqlite-tests.el b/test/src/sqlite-tests.el
index a10dca9a0c9..e87a5fc77b1 100644
--- a/test/src/sqlite-tests.el
+++ b/test/src/sqlite-tests.el
@@ -261,4 +261,30 @@ sqlite-returning
 		        '("Joe" "Doe"))
         '((1 "Joe")))))))
 
+(ert-deftest sqlite-multiple-statements ()
+  (skip-unless (sqlite-available-p))
+  (let ((db (sqlite-open nil))
+        (query (with-temp-buffer
+                 (insert "-- -*- sql-product: sqlite -*-
+
+-- I 💘 emojis
+
+CREATE TABLE settings (
+  name TEXT NOT NULL,
+  value TEXT,
+  section TEXT NOT NULL,
+  PRIMARY KEY (section, name)
+);
+
+CREATE TABLE tags📎 (
+  name TEXT PRIMARY KEY NOT NULL
+);
+
+-- CREATE TABLE todo_states (id INTEGER PRIMARY KEY, name TEXT NOT NULL);
+")
+                 (buffer-string))))
+    (sqlite-execute-batch db query)
+    (should (equal '(("settings") ("tags📎"))
+                   (sqlite-select db "select name from sqlite_master where type = 'table' and name not like 'sqlite_%' order by name")))))
+
 ;;; sqlite-tests.el ends here
-- 
2.29.2.154.g7f7ebe054a


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#70145: [PATCH] Add sqlite-execute-batch command
  2024-04-14  3:32               ` Javier Olaechea
@ 2024-04-14  5:52                 ` Eli Zaretskii
  0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2024-04-14  5:52 UTC (permalink / raw)
  To: Javier Olaechea; +Cc: 70145, jp

> From: Javier Olaechea <pirata@gmail.com>
> Date: Sat, 13 Apr 2024 22:32:05 -0500
> Cc: jp@neverwas.me, 70145@debbugs.gnu.org
> 
>  Also, to accept this contribution, we'd need a copyright assignment
>  from you.  Would you like to start the paperwork for that at this
>  time?  If yes, I will send you the form to fill and the instructions
>  to go with it.
> 
> Yes, please send the form please.

Form sent off-list.





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-04-14  5:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-02 15:03 bug#70145: [PATCH] Add sqlite-execute-batch command Javier Olaechea
2024-04-02 16:21 ` Eli Zaretskii
2024-04-02 18:52   ` Javier Olaechea
2024-04-02 18:56     ` Eli Zaretskii
2024-04-02 19:10       ` Javier Olaechea
2024-04-03  1:56         ` J.P.
2024-04-03  2:42           ` Javier Olaechea
2024-04-13  8:03             ` Eli Zaretskii
2024-04-14  3:32               ` Javier Olaechea
2024-04-14  5:52                 ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).