Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] use single-quotes for string literals in sqlite queries #5419

Closed
wants to merge 1 commit into from

Conversation

amonks
Copy link

@amonks amonks commented Sep 13, 2024

Historically, sqlite accepts double-quoted string literals, which is in contravention of the sql standard (which says single quotes should be used for string literals, and double quotes for identifiers like column names).

The sqlite authors consider this a misfeature, and are making efforts to change it:

  • Since sqlite 3.27.0 (2019-02-07), using double-quotes for a string literal causes a warning to be printed to the error log.
  • Since sqlite 3.29.0 (2019-07-10), the sqlite authors recommend compiling sqlite with the -DSQLITE_DQS=0 option, which causes use of double quotes for string literals to be an error.

More details here: https://www.sqlite.org/quirks.html#double_quoted_string_literals_are_accepted

My sqlite is compiled with this recommended option, so prior to this change, running, eg, beet ls, produced this error:

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 1094, in _fetch
    rows = tx.query(sql, subvals)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/dbcore/db.py", line 859, in query
    cursor = self.db._connection().execute(statement, subvals)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.OperationalError: no such column: "" - should this be a string literal in single-quotes?

Making the appropriate change to library.py allowed beet ls to continue to this error:

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: "text" - should this be a string literal in single-quotes?

The change to query.py allowed beet ls to succeed.

As for testing, I frankly don't think it's worthwhile to build an automated test harness for beets that recompiles sqlite differently. I think it's sufficient in this case to observe that the existing tests pass. I've been running with this patch for 6 months or so, and haven't had any issues.

Description

Fixes #X.

(...)

To Do

  • Documentation. (If you've added a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
  • Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)
  • Tests. (Very much encouraged but not strictly required.)

Historically, sqlite accepts double-quoted string literals, which is in
contravention of the sql standard (which says single quotes should be
used for string literals, and double quotes for identifiers like column
names).

The sqlite authors consider this a misfeature, and are making efforts to
change it:
- Since sqlite 3.27.0 (2019-02-07), using double-quotes for a string
  literal causes a warning to be printed to the error log.
- Since sqlite 3.29.0 (2019-07-10), the sqlite authors recommend
  compiling sqlite with the -DSQLITE_DQS=0 option, which causes use of
  double quotes for string literals to be an error.

More details here: https://www.sqlite.org/quirks.html#double_quoted_string_literals_are_accepted

My sqlite is compiled with this recommended option, so prior to this
change, running, eg, `beet ls`, produced this error:

    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 1094, in _fetch
        rows = tx.query(sql, subvals)
               ^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/local/lib/python3.11/site-packages/beets/dbcore/db.py", line 859, in query
        cursor = self.db._connection().execute(statement, subvals)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    sqlite3.OperationalError: no such column: "" - should this be a string literal in single-quotes?

Making the appropriate change to library.py allowed `beet ls` to
continue to this error:

    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: "text" - should this be a string literal in single-quotes?

The change to query.py allowed `beet ls` to succeed.

As for testing, I frankly don't think it's worthwhile to build an
automated test harness for beets that recompiles sqlite differently. I
think it's sufficient in this case to observe that the existing tests
pass. I've been running with this patch for 6 months or so, and haven't
had any issues.
@arogl
Copy link
Contributor

arogl commented Sep 13, 2024

Issue here #4709 Avoid using double-quoted string literals in SQL queries

Existing PR here #5236

@amonks
Copy link
Author

amonks commented Sep 16, 2024

@arogl right you are! I'll close this.

@amonks amonks closed this Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants