Allow wildcards with fields! Fixes #3978

[corylanou]

We previously had checks to disallow mixing fields and wildcards as the functionality was not wired up. We now have the ability to expand wildcard properly even while mixing them with fields/tags. This PR removes the restriction previously added.

[otoolep]

I don't follow why this was removed. Can you explain?

[corylanou]

This previously assumed a single wildcard, and they returned. so something like select *, foo would fail, but select foo, * would pass. In general, it's code bloat now and the code below handles a wildcard fine.

[otoolep]

I think this needs a single query as a test in server_test.go, no?

[otoolep]

Change generally makes sense, but I'd like slightly improved test coverage before I'd feel 100%.

[corylanou]

@otoolep I had them in my buffer and forgot to save them gah! Here they are. Thanks!

[otoolep]

+1

[beckettsean]

@corylanou if possible can you check the queries from #3327 against the new code? Those are all the queries in our docs, as of a month ago. Given a major change like this I want to make sure they all still work.

[corylanou]

@beckettsean I think what you are asking me to do is complete #3327 :-). That is beyond the scope of this issue, but I agree, they should all work. We are missing all the data to go with those queries and their results. I do agree we should have that code coverage, but it is going to take some effort to complete that task.

In general, this PR puts us further ahead than behind from a functionality standpoint, and we have a lot of test coverage in general around the query language right now. If any bugs do arise from this, they will likely be around the new functionality, not existing functionality.

[beckettsean]

@corylanou mostly I was hoping that with some toy data you can replicate the SELECT * queries from that page, at least in structure. I don't care if they are they exact queries, but I want to be sure that SELECT * FROM /.*/ LIMIT 1 still works, for instance.

Pulling them out, here's a generic list:

SELECT * FROM measurement1, measurement2

SELECT * FROM /<regex matching multiple measurements>/ # e.g. FROM /^cpu.*/ for telegraf data

SELECT * FROM /.*/ limit 1;

SELECT * FROM database../foo.*/ LIMIT 5
SELECT * FROM database../foo.*/ SLIMIT 5

SELECT * FROM retention_policy./foo.*/ LIMIT 1

SELECT * FROM measurement WHERE tag =~ /foo/
SELECT * FROM measurement WHERE tag !~ /.*foo.*/
SELECT * FROM events WHERE (tag =~ /.*foo.*/ OR tag =~ /.*foo.*/) AND tag2 = 'foo'

SELECT * FROM measurement WHERE field > something AND tag = 'something'

SELECT * FROM "measurement with unicode"

SELECT * FROM "measurement with \"double quotes\""

[corylanou]

Well since you asked so nice how can I say no? 😄

[DanielMorsing]

+1

[corylanou]

@beckettsean I added some more tests, but not all of them. Due to the way the parser works, the majority of the test cases don't even touch the branches of code I changed. I know this may look like a sweeping change, but due to the way the parser works, this is actually a somewhat minor change.

[beckettsean]

Thanks very much, @corylanou, getting some of those queries anywhere into the unit tests is fantastic! I was really thinking just manual testing on your branch before merging, so you did more than I expected!