-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
SELECT solely on time should return error #3778
Conversation
3229ee4
to
07568fe
Compare
It should actually be valid to put |
@pauldix -- what if it's the only SELECT field? No results back? |
then it should throw an error. They need to actually select some real data. |
Right, makes sense. So if it's the only field throw back an error, if it's accompanied by other fields, ignore it. |
@pauldix -- policy updated, plus test added to show that |
566713f
to
78faf95
Compare
78faf95
to
ff0a52a
Compare
@@ -1394,6 +1394,11 @@ func TestServer_Query_Common(t *testing.T) { | |||
exp: fmt.Sprintf(`{"results":[{"series":[{"name":"cpu","columns":["time","value"],"values":[["%s",1]]}]}]}`, now.Format(time.RFC3339Nano)), | |||
}, | |||
&Query{ | |||
name: "explicitly selecting time and a valid measurement and field should succeed", | |||
command: `SELECT time,value FROM db0.rp0.cpu`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A test with select value, time
would be nice as it would show that time
will always be the first column, but all other columns still maintain their sort order (or should)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, well, that test case fails. The following data comes back:
actual: {"results":[{"series":[{"name":"cpu","columns":["value","time"],"values":[["2015-08-20T23:57:35.973310065Z",1]]}]}]}
I'd like to address this separately as it will require a somewhat wider set of changes. OK? #3779
I'd like to get this panic fixed before the 0.9.3 window closes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good
+1 |
@pauldix are there event based use cases where the value isn't important...only the time it happened matters? |
@dgnorton maybe, but they still need to store at least one value. For those use cases I would recommend using a boolean. Guess it's a little wasteful to return that, but this is definitely the least of our concerns right now. |
+1 |
ff0a52a
to
20e5d5f
Compare
Thanks for the reviews @pauldix and @corylanou -- merging on green and will cherry-pick to 0.9.3. |
SELECT solely on time should return error
FWIW, I'm building an alerting system on top of influxdb and it would be really useful to be able to get the time of the last reported value for a particular series, without particular reference to the value itself. The goal is to write an alert that checks for "is still receiving data", in case the sensors in question die. |
@nornagon that sounds like a valid use case that hasn't been fully discussed. Can you open a new issue with a feature request? |
Done: #4862 |
Fixes #3010.
Simply check for
time
in theSELECT
clause, and kick it back if the query contains it. Should this be a case-insensitive check?