Skip to content
This repository was archived by the owner on Nov 7, 2025. It is now read-only.

Conversation

@nablaone
Copy link
Member

@nablaone nablaone commented Dec 9, 2024

Our queries fail if we try to do sum or avg over a string or date column. This PR detects and compensates (adds a hack) it.

@nablaone nablaone requested a review from a team as a code owner December 9, 2024 12:27
@nablaone nablaone marked this pull request as draft December 9, 2024 12:27
@nablaone nablaone marked this pull request as ready for review December 9, 2024 13:52
@nablaone nablaone changed the title WIP - Fix Avg/Sum over Datetime column Fix Avg/Sum over Datetime column Dec 9, 2024
@trzysiek
Copy link
Member

trzysiek commented Dec 9, 2024

I think we should allow those functions on DateTime*, as they work in Elastic
Screenshot 2024-12-09 at 17 14 52
We even have support for that in code, e.g. in model/metrics_aggregations/common.go - the response json is different for dates than normally. It used to work, we must have some regression.

But anyway I'd do that in another PR. I can do that if you want, should take only a few mins.

Copy link
Member

@trzysiek trzysiek left a comment

Choose a reason for hiding this comment

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

Looks good! But if I understand correctly from the code, we will allow such a request, but will return NULL (result of avg/sum(NULL))?
It's fine, so far we probably can't do better, but I'll also change that soon to return error to Kibana just after parsing query (after #1006 and some follow-up) so that we behave like Elastic

@nablaone nablaone added this pull request to the merge queue Dec 10, 2024
Merged via the queue into main with commit 88033c1 Dec 10, 2024
5 checks passed
@nablaone nablaone deleted the pr-agg-over-datetime branch December 10, 2024 10:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants