fix derivative bug: #3247

[sharang]

@jwilder

submit to master

[jwilder]

@sharang So this is essentially moving the start time back one interval only for derivative in CQs? This might need to fixed in the query engine itself.

@jsternberg Thoughts?

[jsternberg]

It might be reasonable to have all derivatives do this, even ad-hoc queries. If we did that, the query engine itself would have to be modified. Just at a glance, this would likely end up fixing it for continuous queries. It would end up causing a single datapoint older than the resample duration to be considered as part of the end result even though I'm not sure it matters.

[jwilder]

I think this should go in the query engine so that the query returns the same results regardless of where its run.

[jsternberg]

Looking at this a bit more, there seem to be more problems with derivatives in continuous queries. I tried running a simple query with a raw derivative and got this from the validation.

> select derivative(value) from cpu where time >= 0 and time < 30 group by time(10s)
ERR: error parsing query: aggregate function required inside the call to derivative

The error message is not the best as the error message should be aggregate function required inside the call to derivative when a group by interval is present, but this brings about some other issues in general with continuous queries. Even if we fixed the interval to include more time for this, this query doesn't even pass the validation phase.

> CREATE CONTINUOUS QUERY mycq ON mydb BEGIN
  SELECT derivative(value) INTO value_derivative FROM cpu GROUP BY time(10s)
END
ERR: error parsing query: aggregate function required inside the call to derivative

It looks like I added this validation as part of fixing #5286. Even if we removed the group by, the continuous query service just wouldn't run the query since it exits early if you try to have the group by be zero.

It looks like older versions could handle this fine so we probably broke the validation at some point. I'm currently looking into where it is broken.

@jwilder should we be allowing a GROUP BY to be used with a derivative() call? The GROUP BY seems to usually refer to the aggregate call inside of derivative() and the duration argument inside of derivative() specifies the time interval instead.

[jwilder]

@jsternberg Yes, you need a GROUP BY for the aggregate function specified in the derivative call. The time duration in the derivative is used after the aggregate function is run and grouped.

[sharang]

@jwilder I think start-time 'moving' should only in CQ.

Since in CQ, it implies that there is another point before the start-time,
CQ need to use the prior point to calculate the result in current interval.

but in a query, user only requesting results which are calculated using points between start-time and end-time, query engine should not change the start-time.

[sharang]

@jwilder @jsternberg change query engine will make user confuse,
besides, maybe need to change lots of current documentation, and add explanation why query engine change the start-time silently.

[sharang]

@jwilder @jsternberg so, what's next?

[jsternberg]

@sharang I think we're going to revisit derivatives for 0.12 after examining what we think the output should be for these kinds of calls. At the moment, this PR wouldn't resolve the issue as the query within the fixed issue doesn't even work anymore when attempting to create a continuous query as mentioned in a previous comment of mine.

When I have time to write up a more detailed description of the problem, I'll link it here so you can be included in the discussion.

[jsternberg]

I'm closing this since I'm working on a different fix and we're not going to do this by modifying the continuous querier.

Thanks for your help.