tumblr / collins

groovy kind of love
tumblr.github.com/collins
Apache License 2.0
571 stars 99 forks source link

Impossible to search for attributes including a dash (-) via CQL #479

Open discordianfish opened 7 years ago

discordianfish commented 7 years ago

Hi,

looks like it's not possible to query collins for assets with an attribute containing a dash (-) when using CQL. Same for both using the API or the collins UI CQL textfield. Searching for "nix-builder" returns:

$ curl -L -u .... -H 'Accept: text/x-shellscript' http://collins.service.consul:9000/api/assets -G --data-urlencode "query=nix-builder=1"
STATUS="error";
DATA_MESSAGE="Error executing search: Invalid cql Error parsing query: string matching regex `!=|=' expected but `-' found";

I can search by using the attribute parameter attribute=nix-builder;1.

I've also tried to escape the dash by using \- or \\- but neither worked.

william-richard commented 7 years ago

Yeah it looks like the regexes used to parse CQL are using ident from here to define keys: http://lampsvn.epfl.ch/trac/scala/browser/scala/tags/R_2_7_1_final/src/library/scala/util/parsing/combinator/JavaTokenParsers.scala?view=markup#L14 Which is used here to define what a key is: https://github.com/tumblr/collins/blob/master/app/collins/solr/CollinsQueryParser.scala#L79 What is weird to me is that I am able to create and query for attributes with numbers in their keys, which does not fit with the definition of ident I found.

Either way, I agree, this is confusing.

I think collins should accept anything considered to be an unquotedString (https://github.com/tumblr/collins/blob/master/app/collins/solr/CollinsQueryParser.scala#L82) as an attribute key, so I think we should use that instead of ident to parse a key in CQL.

@tumblr/collins does that seem reasonable?

discordianfish commented 7 years ago

@Primer42 Should I submit a PR or does this still need discussion?

byxorna commented 7 years ago

@discordianfish sorry about the super delay. If you could submit a PR, that would be great! Make sure to tag me for review :)

discordianfish commented 7 years ago

And here too. Maybe something @davidbirdsong wants to fix.