Skip to content

Conversation

@elffjs
Copy link
Member

@elffjs elffjs commented Jul 24, 2025

No description provided.

Copy link
Contributor

@KevinJoiner KevinJoiner left a 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

Comment on lines +14 to +15
# It's tempting to do all of the float aggregations here, but anything
# involving ordering seems tough.
Copy link
Contributor

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_location

Copy link
Member Author

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.

Copy link
Member Author

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.

Copy link
Contributor

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.

Copy link
Member Author

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).

Copy link
Member Author

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.

Copy link
Member Author

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.

Copy link
Member Author

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FIRST
FIRST
LAST

FIRST
}

input SignalLocationFilter {
Copy link
Contributor

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.

Copy link
Member Author

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.

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.

3 participants