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

Sorting support in beets query mecanism (dbcore) #749

Closed
PierreRust opened this issue May 6, 2014 · 16 comments
Closed

Sorting support in beets query mecanism (dbcore) #749

PierreRust opened this issue May 6, 2014 · 16 comments
Labels
feature features we would like to implement

Comments

@PierreRust
Copy link
Contributor

The creation of this issue has been discussed in #736.

To implement correctly sorting support in the HTTP API (#736) , we need to add support for sorting directly into the query system inside the core (currently, sort order is hardcoded library#albums and library#items).
It would also be nice if users could easily type sorting options as part of queries, even on the command line.

@sampsyo sampsyo added the feature label May 6, 2014
@sampsyo
Copy link
Member

sampsyo commented May 6, 2014

One proposal for human-writable sorting options: prefix the field with a / for ascending sort and \ for descending sort (e.g., /artist would sort artists A-Z). These characters sort of look like the triangles that typically indicate sort order. 😃

Of course, there are obvious problems with typing backslashes in most shells and forward slashes indicate paths, so maybe we need a better idea.

@PierreRust
Copy link
Contributor Author

json-api, and sevarl other sources, suggests using - for desceding sort : sort=-created,title

I was thinking we could use the same convention, and add a "+" for ascending sort, allowing comma-separated fields to support multiple sort criteria.

For example, listing tracks with genre "pop", sorting by artist (ascending) then by year (descending) :

beet list genre:pop +artist,-year

We could also add a 'sort' indicator like this beet list genre:pop sort=+artist,-year, but I don't think it's really necessary.

@sampsyo
Copy link
Member

sampsyo commented May 6, 2014

Not a bad approach at all. One tricky bit is that -title all by itself on the command line will look like flags, and we might want to reserve - for the NOT operator in the future. So maybe we could use +-title for descending sort if that doesn't look too silly.

@PierreRust
Copy link
Contributor Author

hum, I dont' really like +-title, looks confusing to me.

What about prefixing sort criteria with a character, maybe > ?

beet list genre:pop >+artist,-year

@Kraymer
Copy link
Contributor

Kraymer commented May 6, 2014

or using the sort character as suffix?

beet list genre:pop artist+,year-

@PadraicODonoghue
Copy link
Contributor

Suffix seems like a good choice.

It makes the query read alot like natural language.

Beet list tracks where Genre equals Pop, sort by Artist Ascending, Year
Descending

On 6 May 2014 21:30, Fabrice L. notifications@github.com wrote:

or using the sort character as suffix?

beet list genre:pop artist+,year-


Reply to this email directly or view it on GitHubhttps://github.com//issues/749#issuecomment-42354730
.

@sampsyo
Copy link
Member

sampsyo commented May 6, 2014

Yeah, I like the suffix idea! Seems unlikely to conflict with any other meanings.

I actually don't think the comma is really necessary. It seems easy enough to parse genre:pop artist+ year- with the sort fields as separate tokens (in order).

@PadraicODonoghue
Copy link
Contributor

Yeah, the commas we only intended for the "translation" not the actual query

On 6 May 2014 23:51, Adrian Sampson notifications@github.com wrote:

Yeah, I like the suffix idea! Seems unlikely to conflict with any other
meanings.

I actually don't think the comma is really necessary. It seems easy enough
to parse genre:pop artist+ year- with the sort fields as separate tokens
(in order).


Reply to this email directly or view it on GitHubhttps://github.com//issues/749#issuecomment-42369820
.

@PierreRust
Copy link
Contributor Author

For information, I have a first implementation in my repo. It's far from complete and only supports fixed fields for now but I'd like to share to gather first impressions on the general direction.

What I want to implement now :

  • add a default sort order (to maintain the current behavior), ideally configurable
  • add sorting on flexattr (should be doable in sql)
  • add sorting on computed fields (in that case, the sort must be done in python)
  • allow plugins to define their own sort order

@sampsyo
Copy link
Member

sampsyo commented May 23, 2014

Looks good to me so far!

As far as the API goes, it looks like get_query now returns a pair of a Query and Sort object. (Sort, of course, is the new base class.) That makes sense to me. The only other practical alternative I can think of is having Queries themselves include information about sorting, but that seems no better than the current design.

Thanks for looking into this!

@sampsyo
Copy link
Member

sampsyo commented May 25, 2014

I did a little refactoring today that moved most of the query parsing code to a new beets.dbcore.queryparse module. Sorry if this made the merge more difficult. 😳

@PierreRust
Copy link
Contributor Author

I all made some (slow...) progress on this. This branch has now support for fixed and flex fields sorting, both implemented in sql.

still missing :

  • add a default sort order on artist sort field (to maintain the current behavior)
  • make default sort order configurable
  • add sorting on computed fields (in that case, the sort must be done in python)
  • allow plugins to define their own sort order
  • write some documentation
  • write some more test cases

@sampsyo
Copy link
Member

sampsyo commented Jun 11, 2014

Awesome! Does this:

support for fixed and flex fields sorting, both implemented in sql

mean that querying flex fields is also now implemented in SQL? Impressive either way.

@PierreRust
Copy link
Contributor Author

Querying flex flied is indeed implemented in SQL, but only for sorting purpose.

It could be extended for generic queries (that is, to populate item objects) but only if the list of fields to retrieve is given. Otherwise you need another query to get the list of flexattr key, which kind of defeat the purpose ...

Still, we could extract the list of flexattr key from the template and them query them directly in SQL.

@PierreRust
Copy link
Contributor Author

Ok, it's almost complete now :)
Plugin-defined Sort objects are not yet implemented but I think this could be done later.

@sampsyo
Copy link
Member

sampsyo commented Sep 16, 2014

Woohoo; done!

@sampsyo sampsyo closed this as completed Sep 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature features we would like to implement
Projects
None yet
Development

No branches or pull requests

4 participants