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

Remove scalar/scalar comparisons #482

Closed
brian-brazil opened this Issue Jan 27, 2015 · 13 comments

Comments

Projects
None yet
3 participants
@brian-brazil
Copy link
Member

brian-brazil commented Jan 27, 2015

Scalar == scalar is unlike vector == scalar and vector == vector in that it doesn't filter. To keep the language orthogonal and consistent, I propose we drop support for scalar/scalar comparisons. Any case where it would be useful should be doable via vectors.

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Feb 8, 2015

Another option would be to make it filter.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Feb 8, 2015

How would it filter? By returning an empty vector? What purpose would that serve? (but removing sounds good so far)

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Feb 8, 2015

It'd do the same as vector/vector, return the left argument.
Hmm, if we do that we'd want to remove support for scalar/vector and only allow vector/scalar.

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Apr 20, 2015

A use case has appeared for filtering scalar/scalar comparisons: restricting times when alerts fire via math on time(). Accordingly let's filter and take the left argument.

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented May 11, 2015

As complaints about breaking changes are getting louder with time, we might want to make a decision on this in the next few weeks.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented May 21, 2015

@brian-brazil I'm still not clear on how scalar/scalar would filter: if the return type is scalar as well, what would the return value be for something like 5 < 3? NaN?

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented May 21, 2015

Hmm, yeah you'd need to make the result of scalar/scalar comparisons to be vectors for it to work - which would break other things.

That'd imply dropping support for it, and using absent({}) or maybe a one/iota/unity() function that returns a vector with a single no-label sample with value 1 for when you need to filter.

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented May 21, 2015

Devil's advocate - would it make sense to drop scalars all together and parse a number literal as a single-element vector without labels?
Except for arguments in functions such as topk.

We'd have to allow unary expressions for vectors. Label matching simply wouldn't make sense as the label set is empty.

This would naturally solve some issues such as not being able to graph a line when you type 5.
In general that'd be more of an internal change.

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented May 21, 2015

Devil's advocate - would it make sense to drop scalars all together and parse a number literal as a single-element vector without labels?

That'd require group_left/group_right on "scalar"/vector operations, which'd be too overbearing considering how common that is.

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Jul 28, 2015

Another thought is that a scalar/scalar comparison could return a vector, either empty or {}:left value

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Sep 2, 2015

To summarize, we've two options to clean this up:

  • Drop support for non-filtering scalar comparisons
  • Filter, and return a no-label vector with the value

I'm tending towards the first, as that'll leave us the option of doing the second in future.

I'm thinking this should be done the release after #1049 is released, to give users a chance to transition.

@brian-brazil brian-brazil added this to the v0.17.0 milestone Sep 26, 2015

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Sep 26, 2015

0.16 will include bool, so we can drop scalar/scalar in 0.17.

@fabxc fabxc closed this in #1159 Oct 16, 2015

simonpasquier pushed a commit to simonpasquier/prometheus that referenced this issue Oct 12, 2017

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.