Skip to content
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

Add "integral" function to InfluxQL #7591

Closed
wants to merge 3 commits into from
Closed

Add "integral" function to InfluxQL #7591

wants to merge 3 commits into from

Conversation

Tomcat-Engineering
Copy link
Contributor

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
Required only if applicable
  • Provide example syntax

I'll open a pull request against the documentation repository, but essentially this is an aggregator function which returns a single value for the "area under the curve".

You can optionally specify a time interval argument which determines the units of the returned values. This defaults to one second, so

SELECT integral(value) from power_in_watts

will return the energy in units of watt-seconds, whereas

SELECT integral(value, 1h) from power_in_watts

will return the energy in watt-hours.

I think the only potentially contentious/confusing thing is the behaviour at the "edges" of a time period. As for other InfluxQL functions, we only analyse the datapoints within the specified period (or group-by interval). This means that the integral does not include the time between the start of the interval and the first datapoint, nor the time between the last datapoint and the end of the interval.

This will lead to slight inconsistencies, such as a set of daily integral values not quite adding up to the corresponding weekly integral. The only way to solve that would be to look outside the time period being analysed to find the neighbouring points, so that we can interpolate the curve to the edges of the period, but I don't see a mechanism for doing that. It could be potentially very expensive, as you might have to search forwards and back through all time in case there were points to use.

We already have some slight inconsistencies of a similar nature - for example the derivative function doesn't produce a value for the first point in each interval (for the same reason) so a set of derivative series calculated for each day doesn't quite match the result of analysing the whole week at once.

I propose that we just explain the above limitation in the documentation for now.

@Tomcat-Engineering
Copy link
Contributor Author

#1400
#5930

BTW, I propose a separate cumulative_integral function that outputs one value for each datapoint rather than just the aggregate (last) value. I decided that it would be cleaner if this was kept separate from the main integral function. Happy to implement cumulative_integral if people would find it useful.

@Tomcat-Engineering
Copy link
Contributor Author

Documentation pull request is at influxdata/docs.influxdata.com-ARCHIVE#855

@jwilder
Copy link
Contributor

jwilder commented Nov 7, 2016

@jsternberg @nathanielc

@nathanielc
Copy link
Contributor

The "area under the curve" via the trapezoidal rule looks great. I reviewed the implementation of the aggregator and it makes sense to me. The function signature also makes sense to me.

As for the edge cases around the group by time boundaries, @jsternberg do you know if the derivative takes into account the last point of the previous group by interval? I thought I remembered you working on that at some point.

As for the rest of the code of boiler plate QL functions it looked good to me but I'll defer to @jsternberg on any of that code.

@Tomcat-Engineering
Copy link
Contributor Author

Looks like derivative doesn't do anything fancy like looking for the previous point outside the time interval being analysed, it just doesn't emit anything for the first datapoint.

Copy link
Contributor

@jwilder jwilder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a changlog entry added.

@jsternberg
Copy link
Contributor

@nathanielc derivative will try to take into account the previous point, but only when there is a group by interval.

The only concern I have with this is that derivative will eliminate the first point so if there are 10 points, derivative will return 9. I figure that integral, which should be the opposite of derivative, should restore that point. While we wouldn't be able to restore the + c constant for the integral (which isn't a problem to me personally), I don't see how we would restore the first point that was eliminated by derivative.

If this isn't an issue, then I'm fine with this. @nathanielc comments?

@nathanielc
Copy link
Contributor

derivative will try to take into account the previous point, but only when there is a group by interval.

This was my concern, as derivative will correctly take into account group by time boundaries, where the current implementation will skip the trapezoid that spans the group by time boundary.

Specifically for this test case https://github.com/influxdata/influxdb/pull/7591/files#diff-4efc5fdc06e36abc4addef9f831c2345R2677

I would expect the integral over the entire time range to be the same as the sum of the integrals over each group by time interval. The data for the test case is (in line protocol):

m value=20  10
m value=10  15
m value=0   20
m value=-10 30

The integral over the entire range is 15*5+5*5+-5*10 = 50.
The integral over the 20s group by time intervals (as it is currently implemented) is 15*5 = 75 for the first interval and -5*10 = -50 for the second interval. The sum of those two interval is 75+-50 = 25. The 5*5 term is missing and so the result is off of 25 from the integral of the entire interval.

I would expect the behavior to be that the first group by interval value is 15*5 = 75 and the second is 5*5 + -5*10 = -25, then the sum of both intervals is 75+-25 = 50 which is the same value we get if we had performed the integral over the entire interval.

In short just like derivative will carry over the last point from the previous interval if we are grouping by time, I think that integral should do the same. And obviously for the first interval there is nothing to carry over. @Tomcat-Engineering @jsternberg @jwilder Does my expectation make sense? Or would you expect each group by time interval to be independent?

As for the restore the first point question, I don't think it is relevant. An integral is a sum of points, so the integral of n points always returns a single point.

@Tomcat-Engineering
Copy link
Contributor Author

The cumulative_integral function which I proposed in the comments above would be the one which is the inverse of the derivative function. The integral function in this PR is an aggregator, so it returns a single point rather than a time series.

PS - the + c constant doesn't apply in this case because we are dealing with definite integrals.

@Tomcat-Engineering
Copy link
Contributor Author

@nathanielc your expectation makes sense and I'll have a go at implementing it... can anyone point me at the relevant code from derivative? I'm struggling to find the magic bit that handles group-by-time.

@palitu
Copy link

palitu commented Jan 6, 2017

Thanks Tomcat! I have been waiting for an integral component for my energy monitoring. Makes it a bit tough to monitor usage without being able to get the kWh! Im looking forward to adding this to my dashboard, and ultimately using the cumulative_integral when it is added.

cheers!

@bbinet
Copy link
Contributor

bbinet commented Jan 19, 2017

Same here, integral function would be very useful.
What is the status of this PR? Is it planned to be merged soon?

@Tomcat-Engineering
Copy link
Contributor Author

The basic problem is that it is not possible to efficiently find the neighbouring points immediately before and after the queried time interval. These are necessary to give consistent integral results, as discussed above, but you may have to search the whole of time to find them which would be slow.

It may be possible to do clever things to pass points between adjacent group-by-time intervals, but I don't think we would want to do that as the results would be even more inconsistent: a particular time-group could then have four different integral values depending on whether it is at the beginning, middle or end of the query interval, or is the only group in the interval.

Having thought about it for a while, I can't come up with anything better than my original algorithm, despite its limitations. Anyone else got any bright ideas?

@jsternberg
Copy link
Contributor

I think you're right. If we find a solution to that in the future, I assume it won't be a problem for us to add back on that one extra point since it will be at a different time than the previous values.

Can you rebase this and resolve the conflicts? Unfortunately, we're in the weird period where 1.2 has been branched off into its own branch, but we're not merging 1.3 bound PRs to master yet. I also don't have time to look at this in full until probably next week. I'm sorry for the inconvenience. If you can also update the changelog to target 1.3, I'll try to get this merged early next week.

Sorry for taking so long to respond to this.

fn := NewFloatIntegralReducer(interval, groupByTime)
return fn, fn
}
return &floatReduceFloatIterator{input: newBufFloatIterator(input), opt: opt, create: createFn}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does integral() have the same behavior as derivative()? I think this should be using the stream iterator instead. That way, it can get called on every incoming point rather than only after a group by interval is read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

integral returns a single total value aggregating all the incoming points, whereas derivative returns a value for each incoming value, hence the different iterators.

See above discussion of the proposed cumulative_integral function for an integral which returns a point for each incoming point.

@@ -2645,6 +2645,104 @@ func TestSelect_Elapsed_Boolean(t *testing.T) {
}
}

func CheckPoints(t *testing.T, ic *IteratorCreator, query string, expected [][]influxql.Point) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary. I think you can use the same pattern the other tests use. I see that this is used in other parts of the code:

    if err != nil {
        t.Fatal(err)
    } else if a, err := Iterators(itrs).ReadAll(); err != nil {
        t.Fatalf("unexpected point: %s", err)
    } else if !deep.Equal(a, [][]influxql.Point{
        {&influxql.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 0 * Second, Value: 19, Aggregated: 2}},
        {&influxql.FloatPoint{Name: "cpu", Tags: ParseTags("host=B"), Time: 0 * Second, Value: 10, Aggregated: 1}},
        {&influxql.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 10 * Second, Value: 2, Aggregated: 2}},
        {&influxql.FloatPoint{Name: "cpu", Tags: ParseTags("host=A"), Time: 30 * Second, Value: 100, Aggregated: 1}},
    }) {
        t.Fatalf("unexpected points: %s", spew.Sdump(a))
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I just thought that rather than duplicating the same code loads of times it would be best practice to write it once and wrap it in a function - but I'm happy to remove the function and get copy-pasting if that is the preferred style!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, if you can remove this that would be great. I understand the desire and I would consider a PR that changes everything for this pattern, but I don't want to mix test styles at the moment. When I've tried that in the past, it became unruly and I had to undo all of it.

@jsternberg
Copy link
Contributor

I'm afraid I might be a bit confused about the direction of this PR and I think some new features (like subqueries) may also factor into that confusion.

Can you explain to me in words which of the following queries are valid and what they are supposed to look like?

SELECT integral(value) FROM cpu
SELECT integral(mean(value)) FROM cpu
SELECT integral(value) FROM cpu GROUP BY time(5m)
SELECT integral(mean(value)) FROM cpu GROUP BY time(5m)

The queries are abbreviated so just assume they have the proper where clause. If I'm understanding correctly, all of those queries would return one value that would just be the sum of the area under the curve. Is that correct?

I'm also thinking subqueries may influence what we do here. Before 1.2, you would have needed a special function to find the sum of an integral. We had one person who really needed a sum of derivatives functionality for a similar purpose. I'm thinking that we might want to make this function more similar to derivative() though so it acts as an exact opposite. That way, the current implementation would be done with:

SELECT sum(integral) FROM (SELECT integral(value) FROM cpu)

Basically, make the proposed cumulative_integral() into the default integral() and suggest that people use subqueries for the sum of that curve.

Thoughts?

@Tomcat-Engineering
Copy link
Contributor Author

SELECT integral(value) FROM cpu
Join each point in cpu to its neighbours with straight lines. Return the total area under this shape.

SELECT integral(mean(value)) FROM cpu
Doesn't work, both integral and mean are aggregators so this is rejected by the parser.

SELECT integral(value) FROM cpu GROUP BY time(5m)
Within each time-bin, join each point in cpu to its neighbours with straight lines. Return the total area under this shape for each bin.

SELECT integral(mean(value)) FROM cpu GROUP BY time(5m)
In theory this could compute the mean value for each 5-minute bin and then integrate the resulting curve - but the parser doesn't allow similar nested queries like sum(mean(value)) so for consistency I haven't updated the parser to allow this and the query will be rejected.

@Tomcat-Engineering
Copy link
Contributor Author

Perhaps the confusion is mostly about naming - if you expect integral to be the inverse of derivative then we should use my proposed cumulative_integral as the integral function as you suggest. I think the "integral as aggregator" was the feature that people meant when they originally requested this, but I'm happy to change the name if it is less confusing.

However we define it, integral will never be an exact inverse of derivative - apart from anything else, derivative returns one fewer points than it receives so we can never get back to the original data.

I think the current implementation is still useful enough to include, perhaps with the name total_integral or something. Based on the comments in the original feature request, this function is what people are going to use most of the time and it would be annoying to have to use subqueries all the time for this common operation.

This is exactly analogous to the sum function: you could implement this by taking the last value from the cumulative_sum results in a subquery, but the aggregator is used more often so gets its own function.

Likewise, to get the total integral out of the cumulative_integral result you take the last value. You can see where I got the idea for the naming from!

@jsternberg
Copy link
Contributor

Yea, you'll never be able to reverse the derivative completely and you'll still lose a point, but there will always be limitations and integrating in calculus already has the + c thing because you can't retrieve the constant after it has been removed when finding the derivative.

I think changing this to be the proposed cumulative_integral() would give more flexibility though because the current implementation of integral() could be implemented with using the subquery that I mentioned in my last comment.

@nathanielc thoughts? Maybe I'm off-base and I don't understand the use cases very well. I'm also not sure which time the integral should output. I would say the time of the second point rather than the first point (like derivative).

@Tomcat-Engineering
Copy link
Contributor Author

integral has been listed in the documentation as an aggregation for ages, so someone at least was expecting it to return a single point! But happy to rename it to total_integral if you think it is more clear.

@dreamon-dd
Copy link

For the use cases, if I understand the current implementation correctly, this function will basically resolve the need of time-weighted average, described in #7445, if divided by the time difference between last and first point in each interval. The result is still using interpolation between points, not steps for the value change as described in #7445, but will be already much better than in case of using simple mean() which doesn't consider time. Therefore it will be essential for downsampling time-unregular sensor data.

@jsternberg
Copy link
Contributor

I don't think integral will solve any problem of time weighted averages because I don't think integral performs any time weighted average. I think we need to implement the weighted_moving_average() function for that.

I'll have a longer post responding to the specifics of integral in a bit. I need some time to organize my thoughts into understandable words.

@dreamon-dd
Copy link

The value of the integral on the interval divided by the duration of this interval is the average value on this interval, isn't it?

@jsternberg
Copy link
Contributor

@Tomcat-Engineering

I talked with @pauldix a bit today about the integral() function and we came back to looking at the function in graphite so we could be certain we implemented what was in graphite. Here's a link to their documentation: http://graphite.readthedocs.io/en/latest/functions.html#graphite.render.functions.integral

I'm a bit confused by the description of how integral works here, so I launched a graphite instance and grafana to try some simple graphs to see what these functions did without having to read the documentation. The descriptions are pretty accurate, but they're also confusing because the definition of integral() in the Grafana documentation is not actually what an integral is. It just seems to sum every value that comes in. Now, it doesn't normalize it for time and I don't think you can do that with basic time math in the query so we can talk about a function for doing that, but I'm not sure we should use the name integral(). While it might be confusing for those coming from graphite, I think we should be focusing on implementing integral() and not focusing on implementing what graphite incorrectly thinks an integral is. I looked around for some confirmation about my thoughts here and found this blog post from grafana confirming that graphite is just wrong here.

I would also like to know if cumulative_sum() matches the current function. From what I see, cumulative_sum() should implement the same functionality as graphite's integral(), but without the time normalization. Maybe we can add an argument to cumulative_sum() for it to normalize by time or think of a function name to do that.

Going back to the difference between aggregate and transformation, I don't think an aggregate is correct for integral. I think we put that down in the documentation before anybody took a serious look at implementing it or deciding what that meant. An aggregate is something that will either be used on the entire span of the series and produce one value (usually without skipping values like derivative sometimes does) or it will perform that same operation on disparate intervals. Aggregates don't consider other intervals when calculating the current interval. I don't think that applies to integral and grafana categorizes both derivative and integral for graphite as transformations. So I think we should be thinking of integral as a transformation rather than an aggregate.

I think an initial question to answer is this: should integral in InfluxDB match integral in Graphite or should we implement an actual integral?

I'm also going to write up a follow-up of how I think integral should be implemented, but I wanted to write this to get some initial thoughts so we could be on the same page.

@pauldix
Copy link
Member

pauldix commented Mar 17, 2017

Thanks for the writeup @jsternberg. Seeing this I think we should implement an actual integral function. Sounds like we already have the equivalent to Graphite's function with cumulative_sum.

@jsternberg
Copy link
Contributor

A few more comments on this and how we should interpret the query. I think we need to define what integral should encompass and determine the syntax. While it doesn't fit into an aggregate, it may also not be the same as derivative and I would like to explore that a bit.

The integral is the area under the curve, but we also need to figure out which part of the area under the curve we're referring to. If we have an example where there are 100 points equally spaced apart, is it desirable to find the area between each of these points? That would result in 99 different areas. There's also the question of whether or not we're finding the cumulative sum of these areas. If you were to use an integral to reverse a derivative, the function that would graph that would be the cumulative sum of these areas. So if the original function is x = t and the derivative is v = 1, then the point at t = 1 would be the integral from 0 to 1 while the points at t = 2 would be the integral from 0 to 2.

So the first question. Should the integral be the cumulative sum of the areas or should it just be the area of the current interval? My preference is that it is the area of the current interval (so from t = 0 to t = 1 and from t = 1 to t = 2 and etc). If you want to get the cumulative sum of the areas, you can just use a subquery to find the cumulative sum from the output of integral.

The next question. If you ask for an integral of a point and specify nothing else, should this produce a single value (the area under the entire curve) or should it produce the area under the curve between each point? The former might be what is expected by users, but the latter is more flexible because you can use the latter with a subquery and sum() to find the former, but you can't do the same with the latter.

That also ties into bucketing. If we want to find the area under the curve for every 10 minute interval, we could use a subquery with a GROUP BY time(...) to implement that if the area of each section under the curve is returned. Unfortunately, if we have a bucket that is partially in one bucket and partially in another, we lose that information if we attempt to do it through a subquery rather than implementing it directly as part of the function.

As confusing as all of that is, I propose two different syntaxes for consideration.

First syntax:

This has integral work as a pseudo-aggregate. It has some properties of streams and some properties of aggregates. It will likely need its own custom implementation since it doesn't neatly fit into either of them.

/* Find the area under the entire curve.
   Internally, this will use the trapezoid rule to calculate the area
   between each point and aggregate all of them together. */
SELECT integral(value, 1s) FROM cpu

/* Find the area for each grouping.
   If the area between two points is partially in one bucket and partially in
   another, linear interpolation is used to divide the two.
*/
SELECT integral(value, 1s) FROM cpu GROUP BY time(1m)

/* Find the integral curve of each grouping by accumulating from previous buckets. */
SELECT cumulative_sum(integral) FROM (SELECT integral(value, 1s) FROM cpu GROUP BY time(1m))

/* It is not possible to find the area between each point with this syntax as a separate bucket. */

Second syntax:

/* Find the area between each point. If there are 100 points, this will emit 99 values. */
SELECT integral(value, 1s) FROM cpu

/* Find the area for each grouping. This will not be able to use linear interpolation when some area is divided. */
SELECT sum(integral) FROM (SELECT integral(value, 1s) FROM cpu) GROUP BY time(1m)

/* Find the area under the entire curve. */
SELECT cumulative_sum(integral) FROM (SELECT integral(value, 1s) FROM cpu)

/* Find the integral curve of each grouping by accumulating from previous buckets. */
SELECT cumulative_sum(sum) FROM (SELECT sum(integral) FROM (SELECT integral(value, 1s) FROM cpu) GROUP BY time(1m))

I'm leaning towards the first. While I think it'll be slightly more complicated to implement because it doesn't have a precedent, I think it will be more clear to users. The second lends itself to using an aggregate within the function better than the first, but I don't like that syntax anyway and I don't consider the syntax necessary now that we have subqueries. The first also has an advantage in that it seems like it matches what people would expect more.

The negative is the first does not make it easy to find the cumulative sum of the integral since it isn't easy to find the dividing line between each of the lines. But you can simulate it easily by just selecting a grouping time like 1s or 10s. Since I'm proposing that it will interpolate the points between the buckets, then you could have points 10 second apart from each other and still use a 1 second grouping since it will interpolate the areas of the buckets.

Thoughts?

@Tomcat-Engineering
Copy link
Contributor Author

Thanks for the notes @jsternberg.

As far as I can tell, with one minor exception your "first syntax" is exactly what I have implemented.

The minor exception is that my algorithm doesn't interpolate across time-bin boundaries in a group-by-time query. This is because it is really hard (or impossible?) to do this efficiently and I think would require re-writing large parts of the query engine. This was discussed quite a bit in previous comments, and I thought that we had come to the conclusion that we should release it as-is for now, with suitable notes in the documentation, because lots of users are waiting for this feature.

PS - your conclusions on Graphite's integral are correct. You based the current InfluxQ cumulative_sum function on someone trying to implement integral by copying Graphite!

@Tomcat-Engineering
Copy link
Contributor Author

PS - I think the idea of using any form of integral to reverse (invert) a derivative is a red herring. I've attached a sketch showing why - apologies for the slightly wonky drawing!

img_0834

The first graph shows a simple time series with four points. Each square on the paper is one unit. The second graph is the result of the existing derivative function. The third graph shows that the output of any attempt to integrate the second graph with linear interpolation is just going to be a load of zeroes. I'm obviously not suggesting that you will always get zeroes, just that the output is rarely likely to be very meaningful or useful.

To get the correct results, we would need to define integral to use a bizarre backwards-sample-and-hold (zero order) interpolation as shown in this version of the second graph, where we interpolate horizontally backwards from each point:

img_0835

If you apply the cumulative version of your emit-lots-of points integrator to this interpolation you get back to the first curve (though the constant of integration is only correct because the first point happened to be at t=0).

However, we will definitely not be popular if we define our general-purpose integral function to use this crazy type of interpolation!

@jsternberg
Copy link
Contributor

Ok, I'll review it with an assumption of the first syntax then. I would like the interpolation (and I don't think it would be too difficult), but let's get this merged. Now that I understand the syntax better, it'll be easier for me to review this.

// this means that we allow the curve we are integrating
// to have step changes in it, but who knows whether the points
// actually arrive in any particular order...?
if r.prev.Time == p.Time {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a potential error location. Points aren't guaranteed to be ordered. There are two ways to resolve this. Either have integral request the points be ordered from the storage engine (see median() in influxql/select.go) or by having this function operate on the entire slice instead (see median() again).

// a timestamp of zero. Within a group-by-time, we can set the time to ZeroTime
// and a higher level will change it to the start of the time group.
func (r *FloatIntegralReducer) Emit() []FloatPoint {
if r.groupByTime {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed. If you use ZeroTime, one of the other iterators takes care of setting the start time correctly. Using 0 would result in the wrong time here.

@@ -882,6 +882,16 @@ func (opt IteratorOptions) ElapsedInterval() Interval {
return Interval{Duration: time.Nanosecond}
}

// IntegralInterval returns the time interval for the integral function.
func (opt IteratorOptions) IntegralInterval() Interval {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks slightly different from DerivativeInterval(). Is there a reason this is different?

@@ -279,7 +279,7 @@ func buildExprIterator(expr Expr, ic IteratorCreator, opt IteratorOptions, selec
opt.Interval = Interval{}

return newHoltWintersIterator(input, opt, int(h.Val), int(m.Val), includeFitData, interval)
case "derivative", "non_negative_derivative", "difference", "moving_average", "elapsed":
case "derivative", "non_negative_derivative", "difference", "moving_average", "elapsed", "integral":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

integral() seems to be different than the rest of these. The rest of these are here because they aren't aggregates so they don't support GROUP BY or fill(). If this is going to be an aggregate, it needs to be moved to the area below this.

@@ -2645,6 +2645,104 @@ func TestSelect_Elapsed_Boolean(t *testing.T) {
}
}

func CheckPoints(t *testing.T, ic *IteratorCreator, query string, expected [][]influxql.Point) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, if you can remove this that would be great. I understand the desire and I would consider a PR that changes everything for this pattern, but I don't want to mix test styles at the moment. When I've tried that in the past, it became unruly and I had to undo all of it.

@jsternberg
Copy link
Contributor

@Tomcat-Engineering I'm going to use your PR as a base for the functionality I described in the first syntax as we discussed before. I'll also be rebasing this onto a more recent commit and making it compatible with subqueries along with trying to implement the interpolation aspect I talked about.

Just giving you a heads up. Is that ok?

@Tomcat-Engineering
Copy link
Contributor Author

@jsternberg - sounds great, thanks a lot and sorry I didn't get around to doing it myself!

@jsternberg
Copy link
Contributor

I've done the fixup and adapted the PR. I'm going to close this one and open up a new PR.

I still want to add the interpolation feature I mentioned above so integral gives more accurate results. I'm hesitant to merge it without that since adding it later will likely count as a breaking change. The difficulty is that, because of that, integral acts differently than both streams and aggregates and it's showing some holes in the method employed to implement both of those. I'll be continuing to work on that, but I wanted to get up what I currently had for the sake of openness.

@jsternberg
Copy link
Contributor

Closing this in favor of #8194.

@jsternberg jsternberg closed this Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants