Remove restrictions on where derivative can be used entirely #6375
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I have reservations about whether some more restrictions should be put in rather than removing them completely, but this can act as a good springboard for that discussion. |
This removes the previous restrictions that kept derivative as only capable of being used in a single field and only at the top level. This lets users determine how they want to use derivative more freely and opens up the possibility of also using math between derivatives. This may open up some problems when it comes to math between derivatives as timestamps may not match correctly. That is likely a problem related to any binary math to begin with though and can probably be ignored by the derivatives. I'm also not sure it makes sense to perform any math between a derivative and a difference or perform math between a derivative and a mean. Fixes #6118.
jsternberg
force-pushed
the
js-6118-derivative-on-multiple-fields
branch
from
b8185a7 to
22a0505
Compare
| derivativeCall := aggr[0] | ||
| if len(derivativeCall.Args) == 0 { | ||
| return fmt.Errorf("derivative requires a field argument") | ||
| } |
There was a problem hiding this comment.
This validation should still exist per call.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the previous restrictions that kept derivative as only
capable of being used in a single field and only at the top level.
This lets users determine how they want to use derivative more freely
and opens up the possibility of also using math between derivatives.
This may open up some problems when it comes to math between derivatives
as timestamps may not match correctly. That is likely a problem related
to any binary math to begin with though and can probably be ignored by
the derivatives. I'm also not sure it makes sense to perform any math
between a derivative and a difference or perform math between a
derivative and a mean.
Fixes #6118.