Skip to content

Commit

Permalink
fix(trends): Fix regressions in formula mode edge cases (#25757)
Browse files Browse the repository at this point in the history
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
Twixes and github-actions[bot] authored Oct 23, 2024
1 parent 1b57c13 commit 4a25a2f
Show file tree
Hide file tree
Showing 9 changed files with 1,320 additions and 1,472 deletions.
13 changes: 12 additions & 1 deletion posthog/hogql_queries/insights/trends/aggregation_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,18 @@ def _math_func(self, method: str, override_chain: Optional[list[str | int]]) ->
else:
chain = ["properties", self.series.math_property]

return ast.Call(name=method, args=[ast.Field(chain=chain)])
return ast.Call(
# Two caveats here:
# 1. We always parse/convert the value to a Float64, to make sure it's a number. This truncates precision
# of very large integers, but it's a tradeoff preventing queries failing with "Illegal type String"
# 2. We fall back to 0 when there's no data, which is not quite kosher for math functions other than sum
# (null would actually be more meaningful for e.g. min or max), but formulas aren't equipped to handle nulls
name="ifNull",
args=[
ast.Call(name=method, args=[ast.Call(name="toFloat", args=[ast.Field(chain=chain)])]),
ast.Constant(value=0),
],
)

def _math_quantile(self, percentile: float, override_chain: Optional[list[str | int]]) -> ast.Call:
if self.series.math_property == "$session_duration":
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Loading

0 comments on commit 4a25a2f

Please sign in to comment.