Skip to content

Commit

Permalink
Update after PR review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
leszko committed Mar 13, 2024
1 parent 77e35a1 commit c3a1e2d
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions views/clickhouse.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ func (c *ClickhouseClient) QueryRealtimeViewsEvents(ctx context.Context, spec Qu
err = c.conn.Select(ctx, &res, sql, args...)
if err != nil {
return nil, err
} else if len(res) > maxClickhouseResultRows {
return nil, fmt.Errorf("query must return less than %d datapoints. consider decreasing your timeframe", maxClickhouseResultRows)

Check warning on line 70 in views/clickhouse.go

View check run for this annotation

Codecov / codecov/patch

views/clickhouse.go#L65-L70

Added lines #L65 - L70 were not covered by tests
}
res = replaceNaNBufferRatio(res)

Check warning on line 72 in views/clickhouse.go

View check run for this annotation

Codecov / codecov/patch

views/clickhouse.go#L72

Added line #L72 was not covered by tests

Expand Down Expand Up @@ -113,7 +115,7 @@ func currentEventsQuery(spec QuerySpec) squirrel.SelectBuilder {
From("viewership_events").
Where("user_id = ?", spec.Filter.UserID).
Where("server_timestamp > (toUnixTimestamp(now() - 30)) * 1000").
Limit(maxClickhouseResultRows)
Limit(maxClickhouseResultRows + 1)

Check warning on line 118 in views/clickhouse.go

View check run for this annotation

Codecov / codecov/patch

views/clickhouse.go#L112-L118

Added lines #L112 - L118 were not covered by tests
}

// timeRangeEventsQuery uses an optimized materialized view "per minute" to favor query time against the latency.
Expand All @@ -127,7 +129,7 @@ func timeRangeEventsQuery(spec QuerySpec) squirrel.SelectBuilder {
Where("user_id = ?", spec.Filter.UserID).
GroupBy("timestamp_ts").
OrderBy("timestamp_ts desc").
Limit(maxClickhouseResultRows)
Limit(maxClickhouseResultRows + 1)

Check warning on line 132 in views/clickhouse.go

View check run for this annotation

Codecov / codecov/patch

views/clickhouse.go#L122-L132

Added lines #L122 - L132 were not covered by tests

if spec.From != nil {

Check warning on line 134 in views/clickhouse.go

View check run for this annotation

Codecov / codecov/patch

views/clickhouse.go#L134

Added line #L134 was not covered by tests
// timestamp_ts is DateTime, but it's automatically converted to seconds
Expand Down

0 comments on commit c3a1e2d

Please sign in to comment.