Skip to content

Commit

Permalink
[fix] makeDateTrunc calculating rolling_month wrong (#52579)
Browse files Browse the repository at this point in the history
Also fixed the function returning a string without a timezone, when it
always returns times in UTC. This is done by explicitly casting to
timestamptz by using `TIMEZONE('UTC', ...)` function.

Added unit tests as well, to make sure everything works as expected.

## Test plan

Unit tests
  • Loading branch information
kopancek authored and ErikaRS committed Jun 22, 2023
1 parent ce9e6e1 commit 63f4e9c
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 4 deletions.
11 changes: 7 additions & 4 deletions internal/database/event_logs.go
Expand Up @@ -1760,16 +1760,19 @@ END

// makeDateTruncExpression returns an expression that converts the given
// SQL expression into the start of the containing date container specified
// by the unit parameter (e.g. day, week, month, or rolling month [prior 30 days]).
// by the unit parameter (e.g. day, week, month, or rolling month [prior 1 month]).
// Note: If unit is 'week', the function will truncate to the preceding Sunday.
// This is because some locales start the week on Sunday, unlike the Postgres default
// (and many parts of the world) which start the week on Monday.
func makeDateTruncExpression(unit, expr string) string {
if unit == "week" {
return fmt.Sprintf(`DATE_TRUNC('%s', TIMEZONE('UTC', %s) + '1 day'::interval) - '1 day'::interval`, unit, expr)
return fmt.Sprintf(`TIMEZONE('UTC', (DATE_TRUNC('week', TIMEZONE('UTC', %s) + '1 day'::interval) - '1 day'::interval))`, expr)
}
if unit == "rolling_month" {
return fmt.Sprintf(`DATE_TRUNC('day', TIMEZONE('UTC', %s)) - '30 day'::interval`, expr)
return fmt.Sprintf(`TIMEZONE('UTC', (DATE_TRUNC('day', TIMEZONE('UTC', %s)) - '1 month'::interval))`, expr)
}

return fmt.Sprintf(`DATE_TRUNC('%s', TIMEZONE('UTC', %s))`, unit, expr)
return fmt.Sprintf(`TIMEZONE('UTC', DATE_TRUNC('%s', TIMEZONE('UTC', %s)))`, unit, expr)
}

// RequestsByLanguage returns a map of language names to the number of requests of precise support for that language.
Expand Down
77 changes: 77 additions & 0 deletions internal/database/event_logs_test.go
Expand Up @@ -2027,3 +2027,80 @@ func TestEventLogs_AggregatedRepoMetadataStats(t *testing.T) {
})
}
}

func TestMakeDateTruncExpression(t *testing.T) {
if testing.Short() {
t.Skip("skipping long test")
}

logger := logtest.Scoped(t)
db := NewDB(logger, dbtest.NewDB(logger, t))
ctx := context.Background()

cases := []struct {
name string
unit string
expr string
expected string
}{
{
name: "truncates to beginning of day in UTC",
unit: "day",
expr: "'2023-02-14T20:53:24Z'",
expected: "2023-02-14T00:00:00Z",
},
{
name: "truncates to beginning of day in UTC, regardless of input timezone",
unit: "day",
expr: "'2023-02-14T20:53:24-09:00'",
expected: "2023-02-15T00:00:00Z",
},
{
name: "truncates to beginning of week in UTC, starting with Sunday",
unit: "week",
expr: "'2023-02-14T20:53:24Z'",
expected: "2023-02-12T00:00:00Z",
},
{
name: "truncates to beginning of month in UTC",
unit: "month",
expr: "'2023-02-14T20:53:24Z'",
expected: "2023-02-01T00:00:00Z",
},
{
name: "truncates to rolling month in UTC, if month has 30 days",
unit: "rolling_month",
expr: "'2023-04-20T20:53:24Z'",
expected: "2023-03-20T00:00:00Z",
},
{
name: "truncates to rolling month in UTC, even if March has 31 days",
unit: "rolling_month",
expr: "'2023-03-14T20:53:24Z'",
expected: "2023-02-14T00:00:00Z",
},
{
name: "truncates to rolling month in UTC, even if Feb only has 28 days",
unit: "rolling_month",
expr: "'2023-02-14T20:53:24Z'",
expected: "2023-01-14T00:00:00Z",
},
{
name: "truncates to rolling month in UTC, even for leap year February",
unit: "rolling_month",
expr: "'2024-02-29T20:53:24Z'",
expected: "2024-01-29T00:00:00Z",
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
format := fmt.Sprintf("SELECT %s AS date", makeDateTruncExpression(tc.unit, tc.expr))
q := sqlf.Sprintf(format)
date, _, err := basestore.ScanFirstTime(db.Handle().QueryContext(ctx, q.Query(sqlf.PostgresBindVar), q.Args()...))
require.NoError(t, err)

require.Equal(t, tc.expected, date.Format(time.RFC3339))
})
}
}

0 comments on commit 63f4e9c

Please sign in to comment.