GNU bug report logs - #65998
Sqlite-mode issue deleting records and closing database

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 65998 in the body.
You can then email your comments to 65998 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thomas Hilke <t.hilke <at> rollomatic.ch>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: Sqlite-mode issue deleting records and closing database
Date: Fri, 15 Sep 2023 09:02:49 +0000

[Message part 1 (text/plain, inline)]

Hi everyone,

I had the opportunity of using sqlite-mode recently, which is really
handy for quickly inspecting the content of a database. However, I
noticed two issues when using it from Windows:

- The sql query built by sqlite-mode-delete is syntactically correct,
  but misinterpreted by sqlite, and eventually do nothing. The query
  is of the form "REMOVE FROM table_name WHERE rowid = ? and
  'column_name_1' = ? and 'column_name_2' = ? and ..."  From
  https://www.sqlite.org/lang_keywords.html, if I understand
  correctly, the quoted column names in the WHERE clause are
  interpreted as strings from sqlite, and as a result not a single row
  is ever matched and deleted.

- The connection to the sqlite database (file) is never closed, even
  when the buffer is killed. As sqlite--db is a local variable,
  it's not even possible to close the connection by hand once the
  buffer is killed. That means that once a database file is opened
  with sqlite-mode-open-file, the file cannot be deleted unless
  emacs is closed (on Windows).

Attached are the modifications that makes it work for me. I didn't have
the opportunity to test it under linux or another environment.

I did not want to touch the other parts that were already working, but
note that sqlite support a collection of pragma statements that can be
used to inspect the schema of the database in a more structured way
than parsing the content of the sqlite_master table:
https://www.sqlite.org/lang_keywords.html.

I hope it helps, and thanks for all the great work!

Thomas Hilke

[Message part 2 (text/html, inline)]

[0001-Remove-column-quoting-and-close-sqlite-db-on-buffer-.patch (application/octet-stream, attachment)]

Message #10 received at 65998 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Thomas Hilke <t.hilke <at> rollomatic.ch>,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 65998 <at> debbugs.gnu.org
Subject: Re: bug#65998: Sqlite-mode issue deleting records and closing database
Date: Sat, 16 Sep 2023 13:24:20 +0300

> Date: Fri, 15 Sep 2023 09:02:49 +0000
> From:  Thomas Hilke via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> I had the opportunity of using sqlite-mode recently, which is really
> handy for quickly inspecting the content of a database. However, I
> noticed two issues when using it from Windows:
> 
> - The sql query built by sqlite-mode-delete is syntactically correct,
>   but misinterpreted by sqlite, and eventually do nothing. The query
>   is of the form "REMOVE FROM table_name WHERE rowid = ? and
>   'column_name_1' = ? and 'column_name_2' = ? and ..."  From
>   https://www.sqlite.org/lang_keywords.html, if I understand
>   correctly, the quoted column names in the WHERE clause are
>   interpreted as strings from sqlite, and as a result not a single row
>   is ever matched and deleted.
> 
> - The connection to the sqlite database (file) is never closed, even
>   when the buffer is killed. As sqlite--db is a local variable,
>   it's not even possible to close the connection by hand once the
>   buffer is killed. That means that once a database file is opened
>   with sqlite-mode-open-file, the file cannot be deleted unless
>   emacs is closed (on Windows).
> 
> Attached are the modifications that makes it work for me. I didn't have
> the opportunity to test it under linux or another environment.

Thanks.  I installed on the emacs-29 branch the first part of your
patch, which closes the DB when the buffer is killed.  As for the
second part, I'd prefer that Lars or someone who knows SQL reviewed it
first, as I find it strange that Lars would code something so basic
which doesn't work at all.

Could someone who knows SQL please review and chime in?

Message #13 received at 65998 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Thomas Hilke <t.hilke <at> rollomatic.ch>, 
 Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 65998 <at> debbugs.gnu.org
Subject: Re: bug#65998: Sqlite-mode issue deleting records and closing database
Date: Sat, 16 Sep 2023 07:07:51 -0700

Eli Zaretskii <eliz <at> gnu.org> writes:

>> - The sql query built by sqlite-mode-delete is syntactically correct,
>>   but misinterpreted by sqlite, and eventually do nothing. The query
>>   is of the form "REMOVE FROM table_name WHERE rowid = ? and
>>   'column_name_1' = ? and 'column_name_2' = ? and ..."  From
>>   https://www.sqlite.org/lang_keywords.html, if I understand
>>   correctly, the quoted column names in the WHERE clause are
>>   interpreted as strings from sqlite, and as a result not a single row
>>   is ever matched and deleted.
>
> Thanks.  I installed on the emacs-29 branch the first part of your
> patch, which closes the DB when the buffer is killed.  As for the
> second part, I'd prefer that Lars or someone who knows SQL reviewed it
> first, as I find it strange that Lars would code something so basic
> which doesn't work at all.
>
> Could someone who knows SQL please review and chime in?

Something like this in an SQL "REMOVE FROM table_name WHERE {foo}" clause

    'column_name_1' = ?

will check if the '?' part is equal to the string 'column_name_1', which
is probably not what we want.

Whereas this

    column_name_1 = ?

will instead check if the '?' part is equal to the value of the column
column_name_1 in table_name.

(The "?" is just a placeholder that will be filled in with an actual
value later.)

So without having tested the code or studied it in detail, the analysis
of the problem sounds right to me.

Don't we have unit tests in place for this stuff, though?  Perhaps we
should see this as an opportunity to add some...

Message #18 received at 65998-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 65998-done <at> debbugs.gnu.org, larsi <at> gnus.org, t.hilke <at> rollomatic.ch
Subject: Re: bug#65998: Sqlite-mode issue deleting records and closing database
Date: Sun, 17 Sep 2023 13:05:47 +0300

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Sat, 16 Sep 2023 07:07:51 -0700
> Cc: 65998 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> - The sql query built by sqlite-mode-delete is syntactically correct,
> >>   but misinterpreted by sqlite, and eventually do nothing. The query
> >>   is of the form "REMOVE FROM table_name WHERE rowid = ? and
> >>   'column_name_1' = ? and 'column_name_2' = ? and ..."  From
> >>   https://www.sqlite.org/lang_keywords.html, if I understand
> >>   correctly, the quoted column names in the WHERE clause are
> >>   interpreted as strings from sqlite, and as a result not a single row
> >>   is ever matched and deleted.
> >
> > Thanks.  I installed on the emacs-29 branch the first part of your
> > patch, which closes the DB when the buffer is killed.  As for the
> > second part, I'd prefer that Lars or someone who knows SQL reviewed it
> > first, as I find it strange that Lars would code something so basic
> > which doesn't work at all.
> >
> > Could someone who knows SQL please review and chime in?
> 
> Something like this in an SQL "REMOVE FROM table_name WHERE {foo}" clause
> 
>     'column_name_1' = ?
> 
> will check if the '?' part is equal to the string 'column_name_1', which
> is probably not what we want.
> 
> Whereas this
> 
>     column_name_1 = ?
> 
> will instead check if the '?' part is equal to the value of the column
> column_name_1 in table_name.
> 
> (The "?" is just a placeholder that will be filled in with an actual
> value later.)
> 
> So without having tested the code or studied it in detail, the analysis
> of the problem sounds right to me.

Thanks, so I've now installed the other part on the emacs-29 branch,
and I'm closing the bug.

> Don't we have unit tests in place for this stuff, though?  Perhaps we
> should see this as an opportunity to add some...

I agree, patches are welcome.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.