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

Define groupBy behavior with output fields named __time #3687

Closed
gianm opened this issue Nov 12, 2016 · 2 comments
Closed

Define groupBy behavior with output fields named __time #3687

gianm opened this issue Nov 12, 2016 · 2 comments

Comments

@gianm
Copy link
Contributor

gianm commented Nov 12, 2016

See #3683, #3684, #3685, which brought to light issues that arise when you have an output field named __time in a groupBy query.

The heart of the issue is that groupBy results have anonymous timestamps in addition to actual named fields coming from dims and metrics and postaggregators. groupBy queries also let you define operations that act on these rows (limit, having, postaggregators, nested queries) and those things are inconsistent with how they act when you ask them to use a field named "__time".

Some issues are,

  • nested queries: If the inner query has a dimension with outputName __time, and the outer query filters or groups on __time.
  • limit specs: If a query has a metric named __time and a limit spec that requests ordering on __time.
  • having specs: If a query has a metric with output name __time, and a having spec filters on __time.

In all those situations the question is: is that referring to the timestamp of result rows or is it referring to the output field named __time? I think that currently (in master) the answers are:

  • nested queries: filters, dimensionSpecs, etc, refer to the timestamp, not the output field.
  • limit specs: refers to the output field, not the timestamp.
  • having specs: refers to the output field, not the timestamp.

So it's not really consistent.

One solution is to define a behavior and stick with it. Another solution is to outlaw output fields named __time. Either of those probably has to wait for 0.10.0 so I'm tagging this issue with that milestone.

@gianm
Copy link
Contributor Author

gianm commented Nov 30, 2016

After some reflection my preferred solution is to outlaw output fields named __time.

@gianm gianm modified the milestones: 0.10.0, 0.10.1 Jan 6, 2017
@gianm
Copy link
Contributor Author

gianm commented Jan 6, 2017

Moving to 0.10.0, incompatible stuff has to be in a 0.x.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant