beets icon indicating copy to clipboard operation
beets copied to clipboard

Avoid using double-quoted string literals in SQL queries

Open TheRealDev0 opened this issue 1 year ago • 8 comments

Problem

With a default beets installation (via pkg or ports) on FreeBSD (13.1), any action requiring interaction with the SQLite database will cause beets to crash.

This is due to Double-quoted String Literals being disabled by default in the 3.41.0,1 version of SQLite - enabling DQS in the make options for the SQLite port corrects this issue and allows beets to complete database functions normally.

The maintainers of the SQLite port on FreeBSD have since re-enabled DQS by default but are planning to disable DQS by default no later than 20240101.

Running this command in verbose (-vv) mode:

$ beet -vv ls

Led to this problem:

Traceback (most recent call last):
  File "/usr/local/bin/beet", line 33, in <module>
    sys.exit(load_entry_point('beets==1.6.0', 'console_scripts', 'beet')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/ui/__init__.py", line 1285, in main
    _raw_main(args)
  File "/usr/local/lib/python3.11/site-packages/beets/ui/__init__.py", line 1272, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/usr/local/lib/python3.11/site-packages/beets/ui/commands.py", line 1089, in list_func
    list_items(lib, decargs(args), opts.album)
  File "/usr/local/lib/python3.11/site-packages/beets/ui/commands.py", line 1084, in list_items
    for item in lib.items(query):
                ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/library.py", line 1529, in items
    return self._fetch(Item, query, sort or self.get_default_item_sort())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/library.py", line 1503, in _fetch
    return super()._fetch(
           ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/dbcore/db.py", line 1093, in _fetch
    rows = tx.query(sql, subvals)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/dbcore/db.py", line 858, in query
    cursor = self.db._connection().execute(statement, subvals)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.OperationalError: no such column:

Setup

  • OS: FreeBSD 13.1
  • Python version: 3.9 and 3.11
  • beets version: 1.6.0
  • Turning off plugins made problem go away (yes/no): no

My configuration (output of beet config) is:

library: library.db
directory: /mnt/Music/
import:
   copy: yes
   move: no

TheRealDev0 avatar Mar 14 '23 15:03 TheRealDev0

Wow; thank you for pointing this out! This looks very serious. For a little more background on the issue, the SQLite docs discuss the syntax compatibility problem: https://www.sqlite.org/quirks.html#double_quoted_string_literals_are_accepted

Unfortunately, it is going to be tricky in general to track down where we are using this incorrect syntax. Based on the clue from this error message, suggesting that the sting literal in question is empty, I found one such instance: https://github.com/beetbox/beets/blob/e51206ae168552983d8b4292d8af49da43fb0886/beets/library.py#L274

That should be '' instead of "". But it's hard to tell if that's the only issue. We should start here, though, and hopefully run the whole test suite with DQS disabled to see if there are more similar crashes.

sampsyo avatar Mar 24 '23 23:03 sampsyo

[...] and hopefully run the whole test suite with DQS disabled to see if there are more similar crashes.

Unfortunately, sqlite3_db_config does not appear to be exposed in any way in Python, so we can't disable DQS at runtime :/

wisp3rwind avatar Mar 25 '23 12:03 wisp3rwind

[...] and hopefully run the whole test suite with DQS disabled to see if there are more similar crashes.

Unfortunately, sqlite3_db_config does not appear to be exposed in any way in Python, so we can't disable DQS at runtime :/

I'm not a Python programmer and I find the namespace and pypi more confusing than dealing with any other languages repositories, but I came across this:

https://docs.python.org/3/library/sqlite3.html#sqlite3.Connection.setconfig

Which seems to expose the thing that's needed for this to work on FreeBSD.

con.setconfig(sqlite3.SQLITE_DBCONFIG_DQS_DDL, 1)
con.setconfig(sqlite3.SQLITE_DBCONFIG_DQS_DML, 1)

That should work on all version of sqlite3 if I'm reading the docs correctly.

UPDATE: Of course, import sqlite3 isn't the same as the docs for this import sqlite3.. This is one of the bigger issues I have with python. There's no way for me to look at a script and figure out which of the things import sqlite3 means.

Nevermind, whatever is satisfying import sqlite3 for the beets project doesn't expose Connection.setconfig(). Yay!

reyjrar avatar May 04 '24 17:05 reyjrar

FreeBSD appears to have disabled DQS again as foretold. Is there any change I can locally apply to get my setup running again? Looks like the setconfig bits aren't it, yeah?

sungo avatar May 07 '24 03:05 sungo

That should work on all version of sqlite3 if I'm reading the docs correctly.

UPDATE: Of course, import sqlite3 isn't the same as the docs for this import sqlite3.. This is one of the bigger issues I have with python. There's no way for me to look at a script and figure out which of the things import sqlite3 means.

Nevermind, whatever is satisfying import sqlite3 for the beets project doesn't expose Connection.setconfig(). Yay!

The problem is likely that setconfig was only added in Python 3.12. But we should try to use it conditionally there, which would get us a check by CI which in turn should be sufficient to track down any remaining DQS usage.

EDIT: I'll open a PR for this later.

wisp3rwind avatar May 07 '24 08:05 wisp3rwind

A quick look throughthe code I found the reference @sampsyo found, as well as the following 2 references in beets/dbcore/query.py.

I am having trouble testing on windows running python 3.12.3, so looking to help out. I'll start a discussion about windows testing

arogl avatar May 07 '24 21:05 arogl

If you want to get things working locally on FreeBSD while this issue is still unresolved, you can build sqlite3 from ports with DQS enabled via make config .

FST777 avatar May 08 '24 07:05 FST777

If you want to get things working locally on FreeBSD while this issue is still unresolved, you can build sqlite3 from ports with DQS enabled via make config .

@FST777 my system is using binary packages and I'm not a fan of mixing ports and packages. but that said, there is a less risky approach, turns out. I built a pyenv of python 3.12 last night, installed beets from git, and manually applied https://github.com/beetbox/beets/pull/5235 , setting the values to 1 instead. on freebsd 13.1, that magically got beets up and running again.

sungo avatar May 08 '24 13:05 sungo