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

Maintain the tags of points selected by top() or bottom() when writing the results #8398

Merged
merged 1 commit into from May 23, 2017

Conversation

jsternberg
Copy link
Contributor

@jsternberg jsternberg commented May 17, 2017

When a SELECT ... INTO ... is used with top() or bottom() used with
tags, the points will be written with the tags still intact instead of
converted to fields.

Fixes #7129.

@jsternberg jsternberg force-pushed the js-7129-top-bottom-write-as-tags branch from 78095e1 to d96d74a Compare May 17, 2017 16:35
@jsternberg
Copy link
Contributor Author

I wanted to get this up to allow discussion about it and methods (along with letting those who are interested build the PR themselves), but this explicitly needs tests before it is merged. The existing tests ensure that it doesn't change current behavior for queries, but the new behavior is strange enough that the lack of tests is a blocker to merging.

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.

One small nit, LGTM.

@@ -781,39 +781,47 @@ func IntegerSpreadReduceSlice(a []IntegerPoint) []IntegerPoint {
return []IntegerPoint{{Time: ZeroTime, Value: max - min}}
}

func newTopIterator(input Iterator, opt IteratorOptions, n int) (Iterator, error) {
func newTopIterator(input Iterator, opt IteratorOptions, n int, writeMode bool) (Iterator, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should writeMode be named keepTags? Seems like that is what it indicates as opposed to having multiple values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds fine to me. The name of the variable was rather arbitrary to begin with. I couldn't think of something good.

@jsternberg jsternberg force-pushed the js-7129-top-bottom-write-as-tags branch from 10aae0b to a5bb16e Compare May 23, 2017 19:49
…g the results

When a `SELECT ... INTO ...` is used with `top()` or `bottom()` used
with tags, the points will be written with the tags still intact instead
of converted to fields.
@jsternberg jsternberg force-pushed the js-7129-top-bottom-write-as-tags branch from a5bb16e to 9edf236 Compare May 23, 2017 20:00
@jsternberg jsternberg merged commit 6608f97 into master May 23, 2017
@jsternberg jsternberg deleted the js-7129-top-bottom-write-as-tags branch May 23, 2017 21:48
@jsternberg jsternberg removed the review label May 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants