handle aggregations with 0 intervals #2547
Conversation
|
+1 |
|
The query in your test won't actually work. You can't have two data points at the same nano-second. And this query is asking for all points at the exact nano-second at Zero intervals aren't actually valid for aggregate queries. The correct way to do this query would be: The better thing to do would be to actually return an error if the query has an aggregate function of any kind and a |
|
No interest in being able to support a |
|
@pauldix so the query is the thing being tested (see #2487) so it needs to stay the same. if i have the wrong expected value for the result then that should change. however, (before i change it) it's my understanding that the intent is to eventually allow more than one point at the same exact nano-second. when that's the case wouldn't aggregates of an exact time make sense? |
|
@pauldix i think this may also fix the non-aggregate case, fwiw |
|
@toddboom i'm not sure what you're referring to but i think queries without aggregates go here https://github.com/influxdb/influxdb/pull/2547/files#diff-135f9f554ea5c27580f7c818e4ae53a1R81 which means this doesn't affect them (for good or bad). maybe i'm misunderstanding what you mean. |
|
@neonstalwart nope, i was wrong - i was testing for an old issue that i think was fixed elsewhere. you're correct. |
|
@neonstalwart we don't currently have a plan to enable points at the same nano-second within an individual series. Although I guess we could have points at the same nano-second across multiple series. So if you have a query like that but there are points with multiple tags then it would work. At the very least I'd like to see the test in server integration updated to have two data points written in with different tags at the same nano-second and see if the result is correct (rather than a null result). |
ok, i misunderstood #1920 but i see now how it's different. test updated - waiting for green. |
|
this failure is unrelated to my change... can someone start it again? |
|
on it |
|
@pauldix how's that now? |
| name: "aggregation with no interval", | ||
| query: `SELECT count(value) FROM cpu WHERE time = '2000-01-01 00:00:00'`, | ||
| queryDb: "%DB%", | ||
| expected: `{"results":[{"series":[{"name":"cpu","columns":["time","count"],"values":[["2000-01-01T00:00:00Z",2]]}]}]}`, |
There was a problem hiding this comment.
Where are these 2 points being written in? I assume it's in another test, but I can't find to validate that 2 is the correct number :)
There was a problem hiding this comment.
the test above it writes the points and we are not resetting between tests
There was a problem hiding this comment.
ok, makes sense. At some point we should really be resetting, but that'll have to come later
There was a problem hiding this comment.
yeah, @otoolep suggested to avoid resetting when possible to keep the time of the tests down.
There was a problem hiding this comment.
Yeah, I've seen test resets that wipe that database, and write all the same data again. Seems kinda pointless. Why not just write a set of data for a given related set of tests, and then execute the various queries. Of course, reset is there when you need it, but if we start needlessly resetting before every write, our test-run times will just grow to many minutes.
As always, it's a judgement call.
handle aggregations with 0 intervals
|
Thanks @neonstalwart! |
fixes #2487
i wondered whether or not a 0-length interval should be valid for aggregating but then i considered that if we eventually allow multiple points to be written to the same time then aggregations over 0-length intervals seem valid.
cc @toddboom