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

Implicit path queries in a boolean OR yield a match-all query #1865

Closed
intelfx opened this issue Feb 4, 2016 · 3 comments · Fixed by #4359
Closed

Implicit path queries in a boolean OR yield a match-all query #1865

intelfx opened this issue Feb 4, 2016 · 3 comments · Fixed by #4359
Labels
bug bugs that are confirmed and actionable

Comments

@intelfx
Copy link
Contributor

intelfx commented Feb 4, 2016

This is best illustrated by example:

  • beet list /path/to/library/A — works
  • beet list /path/to/library/B — works
  • beet list /path/to/library/A , /path/to/library/Blists the whole library
  • beet list path:/path/to/library/A , path:/path/to/library/B — works
@sampsyo sampsyo added the bug bugs that are confirmed and actionable label Feb 4, 2016
@sampsyo sampsyo changed the title query: using "fallback-to-path" feature with OR yields a match-all query Implicit path queries in a boolean OR yield a match-all query Feb 4, 2016
sampsyo added a commit that referenced this issue Feb 4, 2016
@sampsyo
Copy link
Member

sampsyo commented Feb 4, 2016

Thanks for the report! This is definitely a bug. I added a unit test for it, which we should enable when it's fixed.

Here's the problem. The beets.library.parse_query_parts function has to play some dirty tricks to special-case implicit path queries. It does this by stripping out path-esque pieces from the query, parsing them separately, and then adding them back into the result of parsing the rest of the query. This worked fine before we added OR queries, but now it (a) leaves blank gaps in the original query, which is the cause of this bug, and (b) adds the path queries back in at the end, which seems messy and might break in the future.

We'll need to rethink how that works. Maybe we need to add a general "implicit query type" functionality to dbcore's query parser.

@intelfx
Copy link
Contributor Author

intelfx commented Feb 4, 2016

It does this by stripping out path-esque pieces from the query, parsing them separately, and then adding them back into the result of parsing the rest of the query.

Hm, why does it need to do such complex things? Isn't it possible to straight-out replace undecorated path-like pieces with proper queries to the path field, in a "preprocessor" manner?

@sampsyo
Copy link
Member

sampsyo commented Feb 4, 2016

Actually, that's not a bad idea! I feel dumb now, but I'd never thought of that, and I don't see any problems with it. Good thinking. ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs that are confirmed and actionable
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants