More for confirmation than a request to change - is it expected to exclude the end_time from the query ?

If I say I want the insights from 01/01to 02/01- should we include 02/01 ?

In-memory DB doesn't match this:

ifuser.CreatedAt.Before(arg.StartTime)||user.CreatedAt.After(arg.EndTime) {

Missing|| user.CreatedAt.Equal(arg.EndTime).

@defelmnq I will double-check that.

@mafredri I didn't get your last comment. Sorry 😞

is it expected to exclude the end_time from the query ?

No. Not quite, at least. The intention is to include it. We'll generate a series of dates starting from the start date with 24 hour increments. This means that subject to some offset (<24h), the date of the endtime param will be included.

Will this work correctly with timezones?

In-memory DB doesn't match this:

ifuser.CreatedAt.Before(arg.StartTime)||user.CreatedAt.After(arg.EndTime) {

Missing|| user.CreatedAt.Equal(arg.EndTime).

This might produce strange results if requested start time and end time are in a different timezone. Each day may be missing few/many entries and we might get ghost dates as well (+/-1 day outside start/end range).

Suggestion:

date_trunc('day', (created_at ATTIME ZONE EXTRACT(TIMEZONEFROM @start_time::timestamptz))) ATTIME ZONE EXTRACT(TIMEZONEFROM @start_time::timestamptz)ASdate,

(Untested), why the double at timezone? One strips timezone and the other one adds it back.

coder=# select created_at from users;          created_at------------------------------- 2024-04-08 11:41:35.096834+00 2024-04-08 11:40:54.587231+00(2 rows)coder=# select created_at at time zone 'europe/helsinki' from users;          timezone---------------------------- 2024-04-08 14:41:35.096834 2024-04-08 14:40:54.587231(2 rows)coder=# select created_at at time zone 'europe/helsinki' at time zone 'europe/helsinki' from users;           timezone------------------------------- 2024-04-08 11:41:35.096834+00 2024-04-08 11:40:54.587231+00(2 rows)

Notice +00 and lack thereof. Now notice how we get a timezoneally correct truncation when we do it without timezone:

coder=# select date_trunc('day', created_at) from users;       date_trunc------------------------ 2024-04-08 00:00:00+00 2024-04-08 00:00:00+00(2 rows)coder=# select date_trunc('day', created_at at time zone 'europe/helsinki') at time zone 'europe/helsinki' from users;        timezone------------------------ 2024-04-07 21:00:00+00 2024-04-07 21:00:00+00(2 rows)

You might want to use a subquery so you can count over the improved truncation without repeating all that noise.

This function name is a bit confusing. I know it makes sense for the test but might be better to define it inside the parent test function instead of entirecoderd_test package.

It would be good to add tests with other timezones too. Might need to consider the value of user created at more closely to trigger edge cases.

Tip: One alternative to this method is to generate the empty dates as well viagenerate_series in the query. But don't feel you need to make the change now, just suggesting it as an option for the future.

I will take a look at that.

@mafredri not sure why this golden file was not on main since I didn't change it 🤔

You might find this in your tests too but Zone it not guaranteed to return a name. It's unfortunately safer to use the offset (integer) and turn it into a string like2 => "2" and `-2 => "-2".

That's a bit unfortunate however since there may be a minor edge case during DST shift and could potentially produce an unwanted extra/missing row depending on the data. I haven't tried to verify if/how much of a problem it would be though.

Changed to use offset!

This doesn't behave like intended. You need to use the strconv package instead.

Yep, that does look redundant. Let's remove it in a separate PR.