Enforce the same interface across all \`...Query\` implementations

[snejus]

Description

Make PlaylistQuery a FieldQuery

While working on the DB optimization and looking at updates upstream I discovered one query which does not follow the FieldQuery interface —PlaylistQuery, so I looked into it in more detail and ended up integrating it as a FieldQuery.

One special thing about it is that it uses IN SQL operator, so I added implementation for this sort of query outside the playlist context, see InQuery.

Otherwise, it seems like PlaylistQuery is a field query with a special way of resolving values it wants to query. In the future, we may want to consider moving this kind of custom initialization logic away from init methods to factory/@classmethod: this should make it more clear that the purpose of such logic is to resolve the data that is required to define a particular FieldQuery class fully.

Remove NamedQuery since it is unused

This simplifies query parsing logic in queryparse.py. We know that this logic can only receive FieldQuery classes thus I adjusted types and removed the logic that handles other cases.

Effectively, this means that the query parsing logic does not need to care whether the query is named by the corresponding DB field. Instead, queries like SingletonQuery and PlaylistQuery are initialized with the same data as others and take things from there themselves: in this case they translate singleton and playlist queries to the underlying DB filters.

[wisp3rwind]

Hey, nice to see that you're picking up the work on dbcore again!

I introduced the NamedQuery class at some point when adding typings to the module. Did you check whether this refactoring still passes type checking (i.e. mypy)? While we don't really enforce this (for now), it would be unfortunate to roll it back only to realize later that just FieldQuery doesn't suffice to express the involved types and constructor signatures.

Generally, I agree that construct_query_part has a quite complicated dispatch to the various query constructors. It would be great if that could be expressed in a more concise way.

[snejus]

Did you check whether this refactoring still passes type checking (i.e. mypy)?

It's happy!

mypy runs live in my editor as I code where it's allowed to control my implementation :)

You've done an amazing job designing the generic types for FieldQuery class - you can see here how the pattern supported the new InQuery nicely out of the box.

[snejus]

@wisp3rwind you can see that I replaced memoryview with bytes in PlaylistQuery to query BLOB column Item.path.

Is there a reason we're using BLOB_TYPE = memoryview instead of bytes? I tested querying using bytes and I see to get the expected results

[wisp3rwind]

Is there a reason we're using BLOB_TYPE = memoryview instead of bytes? I tested querying using bytes and I see to get the expected results

https://github.com/beetbox/beets/blob/92fb830559530a4913b6966059c053fdd1e46bcf/beets/library.py#L41-L44

[snejus]

Is there a reason we're using BLOB_TYPE = memoryview instead of bytes? I tested querying using bytes and I see to get the expected results

https://github.com/beetbox/beets/blob/92fb830559530a4913b6966059c053fdd1e46bcf/beets/library.py#L41-L44

Thanks!

This may be slightly outdated (this was introduced ~8 years ago) or am I missing something?

🐶 beet list label:ᴄʟᴇʀɢʏ -f '$path'
/run/media/sarunas/music/Music/🔥/03_daxj_stealthmode.flac

[ins] In [17]: db = sqlite3.connect("/home/sarunas/.music/beets/library.db")

[ins] In [18]: res = list(db.execute("select path from items where path = ?", ("/run/media/sarunas/music/Music/🔥/03_daxj_stealthmode.flac".encode(),)))

[ins] In [19]: res
Out[19]: [(b'/run/media/sarunas/music/Music/\xf0\x9f\x94\xa5/03_daxj_stealthmode.flac',)]

[ins] In [20]: res[0][0].decode()
Out[20]: '/run/media/sarunas/music/Music/🔥/03_daxj_stealthmode.flac'

[wisp3rwind]

Is there a reason we're using BLOB_TYPE = memoryview instead of bytes? I tested querying using bytes and I see to get the expected results

https://github.com/beetbox/beets/blob/92fb830559530a4913b6966059c053fdd1e46bcf/beets/library.py#L41-L44

Thanks!

[...]

This may be slightly outdated (this was introduced ~8 years ago) or am I missing something?

I really don't know. Is there any benefit to changing this? If at all possible, I'd prefer not to do so, at least not without a very good understanding of the original reasoning. Both to keep this PR focused, and because I'm afraid that this has the potential to break things in unexpected ways on different systems (with different OS, filesystem, terminal encodings).

Note that memoryview was specifically introduced for Python 3, so it doesn't look like a leftover of Python 2 times.