-
Notifications
You must be signed in to change notification settings - Fork 2
Loc filt #188
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
base: main
Are you sure you want to change the base?
Conversation
Maybe, at least
Any violation of these is a bad program bug
One integration test left to fix
KevinJoiner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some comments I know this is draft so you can just ignore anything that is irrelevant
| # It's tempting to do all of the float aggregations here, but anything | ||
| # involving ordering seems tough. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean anything not involving ordering?
Do you know if something like this works
SELECT tuple(avg(value_location[0]), avg(value_location[1], avg(value_location[2]) as value_locationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would work but is it super meaningful? It's not clear to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Average I think could be okay, although interpretation of HDOP is kinda odd. Median seems flat-out strange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah maybe just first and last, since that is most likely what they need. How will we handle the Aggregations on the old lat and long signals after we migrate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that’s more or less straightforward: replace avg(value_number) with avg(value_location.latitude).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think average would be fine to include. Average each field, they get what they get.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RAND probably also good. I’ll add all this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, boolean logic spooks me a bit.
| # It's tempting to do all of the float aggregations here, but anything | ||
| # involving ordering seems tough. | ||
| enum LocationAggregation { | ||
| FIRST |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| FIRST | |
| FIRST | |
| LAST |
| FIRST | ||
| } | ||
|
|
||
| input SignalLocationFilter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably want some hdop filtering here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, definitely, I think that will be critical.
No description provided.