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

Handle ambiguous column names in queries involving 'any' field and a relation field #5541

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

snejus
Copy link
Member

@snejus snejus commented Dec 7, 2024

Description

Fixes a sqlite3.OperationalError that occurs when querying any field and a field that only exists in the relation table. For exaple, trying to list albums that contain keyword and contain a track with foo in its title:

beet list -a keyword title:foo

Root Cause

SQLite fails when JOINs contain ambiguous column references. This happened because:

  • any Album field search looks at album, albumartist and genre fields.
  • The second part of the query title:foo queries a field in the items table, which got
    JOINed with albums
  • Some fields (like album) exist in both items and albums tables, thus SQLite couldn't resolve which table's column to use

Changes

  • Centralize query construction in LibModel with consistent table qualification
  • Add methods:
    • field_query() - Creates table-qualified field queries
    • any_field_query() - Creates multi-field OR queries
    • any_writable_media_field_query() - Similar to the above but for BPD / media files
    • match_all_query() - Creates multi-field AND queries
  • Remove AnyFieldQuery in favor of composed OrQuery
  • Add tests for shared field querying

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.)

Unify query creation logic from
- queryparse.py:construct_query_part,
- Model.field_query,
- DefaultTemplateFunctions._tmpl_unique

to a single implementation under `LibModel.field_query` class method.
This method should be used for query resolution for model fields.
In order to include the table name for fields in this query, use the
`field_query` method.

Since `AnyFieldQuery` is just an `OrQuery` under the hood, remove it and
construct `OrQuery` explicitly instead.
@snejus snejus self-assigned this Dec 7, 2024
@snejus snejus requested review from bal-e and JOJ0 December 7, 2024 16:40
@snejus snejus force-pushed the fix-any-query-operational-error branch from f9f54cf to 003afb2 Compare December 7, 2024 16:42
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.

1 participant