COOL-cohort / COOL

the source code of the COOL system
https://www.comp.nus.edu.sg/~dbsystem/cool/
Apache License 2.0
45 stars 16 forks source link

134 process the time in a more fine grained manner #135

Closed hugy718 closed 1 year ago

hugy718 commented 1 year ago

Problem

Refer to issue #134 . This PR is a fix for that

Solution

Internally, action time are integers that is seconds from epoch. Age selection calculates the difference of two action time that is created from their seconds from epoch integer. The age selector already supported calculation of difference in different units, but previously it assumes the time is in the granularity of day. It has been updated.

Fixed a bug in the default DayIntConverter. Previously, the action time parsing do not consider the time zone, which was not a problem when we only consider days. Fixed that by treating the input action time as time in UTC time zone. More time formats can be supported by extending ActionTimeIntConverter interface.

The breaking tests are fixed and all existing tests passes.

hugy718 commented 1 year ago

@liuchangshiye Please add the test that you are working on which uses seconds as age granularity as a unit test later.

hugy718 commented 1 year ago

@liuchangshiye please add a fine-grained processing unit test sample, like using 5 seconds for a cohort selection. Then we can merge this before moving on to handling years and month in a separate PR.