Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: rill time issues around truncate and arithmatic #6509

Merged
merged 4 commits into from
Jan 30, 2025
+113 −54
Merged

fix: rill time issues around truncate and arithmatic #6509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
75d5256
Fix anchor arithmatic being truncated
AdityaHegde Jan 27, 2025
4f1c073
Fix invalid firstMonth in timeutil.Truncate
AdityaHegde Jan 27, 2025
43502d3
Fix lint
AdityaHegde Jan 27, 2025
a4c5e01
Add absolute at modifier time support
AdityaHegde Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 71 additions & 41 deletions runtime/pkg/rilltime/rilltime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var (
{"Latest", "latest"},
{"Watermark", "watermark"},
// this needs to be after Now and Latest to match to them
{"Grain", `[smhdDWQMY]`},
{"Grain", `[sSmhHdDwWqQMyY]`},
// this has to be at the end
{"TimeZone", `{.+?}`},
{"AbsoluteTime", `\d{4}-\d{2}-\d{2} \d{2}:\d{2}`},
Expand Down Expand Up @@ -58,13 +58,18 @@ var (
)
grainMap = map[string]timeutil.TimeGrain{
"s": timeutil.TimeGrainSecond,
"S": timeutil.TimeGrainSecond,
"m": timeutil.TimeGrainMinute,
"h": timeutil.TimeGrainHour,
"H": timeutil.TimeGrainHour,
"d": timeutil.TimeGrainDay,
"D": timeutil.TimeGrainDay,
"w": timeutil.TimeGrainWeek,
"W": timeutil.TimeGrainWeek,
"q": timeutil.TimeGrainQuarter,
"Q": timeutil.TimeGrainQuarter,
"M": timeutil.TimeGrainMonth,
"y": timeutil.TimeGrainYear,
"Y": timeutil.TimeGrainYear,
}
)
Expand All @@ -89,8 +94,8 @@ type TimeAnchor struct {
Now bool `parser:"| @Now"`
Latest bool `parser:"| @Latest"`
Watermark bool `parser:"| @Watermark)"`
Offset *Grain `parser:"@@?"`
Trunc *string `parser:" ('/' @Grain)?"`
Offset *Grain `parser:"@@?"`
isoDuration *duration.StandardDuration
}

Expand All @@ -105,8 +110,8 @@ type Grain struct {
}

type AtModifiers struct {
Offset *Grain `parser:"@@?"`
TimeZone *string `parser:"@TimeZone?"`
AnchorOverride *TimeAnchor `parser:"@@?"`
TimeZone *string `parser:"@TimeZone?"`
}

// ParseOptions allows for additional options that could probably not be added to the time range itself
Expand Down Expand Up @@ -164,12 +169,6 @@ func Parse(from string, parseOpts ParseOptions) (*Expression, error) {
}
}

if rt.End == nil {
rt.End = &TimeAnchor{
Watermark: true,
}
}

return rt, nil
}

Expand All @@ -196,25 +195,18 @@ func ParseCompatibility(timeRange, offset string) error {
}

func (e *Expression) Eval(evalOpts EvalOptions) (time.Time, time.Time, error) {
start := evalOpts.Watermark
if e.End != nil {
if e.End.Latest {
// if end has latest mentioned then start also should be relative to latest.
start = evalOpts.MaxTime
} else if e.End.Now {
// if end has now mentioned then start also should be relative to latest.
start = evalOpts.Now
}
}
anchor, fallbackEndAnchor := e.getAnchor(evalOpts)

start := anchor
if e.Start != nil {
start = e.modify(evalOpts, e.Start, start)
start = e.modify(evalOpts, e.Start, anchor)
}

end := evalOpts.Watermark
if e.End != nil {
end = e.modify(evalOpts, e.End, end)
endAnchor := e.End
if e.End == nil {
endAnchor = fallbackEndAnchor
}
end := e.modify(evalOpts, endAnchor, anchor)

return start, end, nil
}
Expand Down Expand Up @@ -261,11 +253,6 @@ func (e *Expression) modify(evalOpts EvalOptions, ta *TimeAnchor, tm time.Time)
return tm.In(e.timeZone)
}

timeBeforeOffset := tm
if ta.Offset != nil {
tm = ta.Offset.offset(tm)
}

if ta.Trunc != nil {
truncateGrain = grainMap[*ta.Trunc]
isTruncate = true
Expand All @@ -278,12 +265,19 @@ func (e *Expression) modify(evalOpts EvalOptions, ta *TimeAnchor, tm time.Time)
modifiedTime = timeutil.CeilTime(tm, truncateGrain, e.timeZone, evalOpts.FirstDay, evalOpts.FirstMonth)
}

// Add offset after truncate
if e.AtModifiers != nil && e.AtModifiers.Offset != nil {
modifiedTime = e.AtModifiers.Offset.offset(modifiedTime)
// Add local offset after truncate. This allows for `0W+1D`
if ta.Offset != nil {
modifiedTime = ta.Offset.offset(modifiedTime)
modifiedTime = timeutil.TruncateTime(modifiedTime, grainMap[ta.Offset.Grain], e.timeZone, evalOpts.FirstDay, evalOpts.FirstMonth)
}

if isBoundary && modifiedTime.Equal(timeBeforeOffset) && (e.Modifiers == nil || e.Modifiers.CompleteGrain == nil) {
// Add global offset from AtModifiers after truncate
// Only grain offset is applied here. Anchor offsets like `@ now` or `@ latest` are applied to `tm` param
if e.AtModifiers != nil && e.AtModifiers.AnchorOverride != nil && e.AtModifiers.AnchorOverride.Grain != nil {
modifiedTime = e.AtModifiers.AnchorOverride.Grain.offset(modifiedTime)
}

if isBoundary && modifiedTime.Equal(tm) && (e.Modifiers == nil || e.Modifiers.CompleteGrain == nil) {
// edge case where the end time falls on a boundary. add +1grain to make sure the last data point is included
n := 1
g := &Grain{
Expand All @@ -299,6 +293,44 @@ func (e *Expression) modify(evalOpts EvalOptions, ta *TimeAnchor, tm time.Time)
return modifiedTime
}

func (e *Expression) getAnchor(evalOpts EvalOptions) (time.Time, *TimeAnchor) {
if e.AtModifiers != nil && e.AtModifiers.AnchorOverride != nil {
if e.AtModifiers.AnchorOverride.Now {
return evalOpts.Now, e.AtModifiers.AnchorOverride
}
if e.AtModifiers.AnchorOverride.Latest {
return evalOpts.MaxTime, e.AtModifiers.AnchorOverride
}
if e.AtModifiers.AnchorOverride.Earliest {
return evalOpts.MinTime, e.AtModifiers.AnchorOverride
}
if e.AtModifiers.AnchorOverride.AbsDate != nil {
absTm, _ := time.Parse(time.DateOnly, *e.AtModifiers.AnchorOverride.AbsDate)
return absTm, e.AtModifiers.AnchorOverride
}
if e.AtModifiers.AnchorOverride.AbsTime != nil {
absTm, _ := time.Parse("2006-01-02 15:04", *e.AtModifiers.AnchorOverride.AbsTime)
return absTm, e.AtModifiers.AnchorOverride
}
}

if e.End == nil {
return evalOpts.Watermark, &TimeAnchor{
Watermark: true,
}
}

if e.End.Latest {
// if end has latest mentioned then start also should be relative to latest.
return evalOpts.MaxTime, e.End
}
if e.End.Now {
// if end has now mentioned then start also should be relative to latest.
return evalOpts.Now, e.End
}
return evalOpts.Watermark, e.End
}

func parseISO(from string, parseOpts ParseOptions) (*Expression, error) {
// Try parsing for "inf"
if infPattern.MatchString(from) {
Expand Down Expand Up @@ -354,23 +386,21 @@ func (g *Grain) offset(tm time.Time) time.Time {
}

switch g.Grain {
case "s":
case "s", "S":
tm = tm.Add(time.Duration(n) * time.Second)
case "m":
tm = tm.Add(time.Duration(n) * time.Minute)
case "h":
case "h", "H":
tm = tm.Add(time.Duration(n) * time.Hour)
case "d":
tm = tm.AddDate(0, 0, n)
case "D":
case "d", "D":
tm = tm.AddDate(0, 0, n)
case "W":
case "W", "w":
tm = tm.AddDate(0, 0, n*7)
case "M":
tm = tm.AddDate(0, n, 0)
case "Q":
case "Q", "q":
tm = tm.AddDate(0, n*3, 0)
case "Y":
case "Y", "y":
tm = tm.AddDate(n, 0, 0)
}

Expand Down
44 changes: 31 additions & 13 deletions runtime/pkg/rilltime/rilltime_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import (
)

func Test_Resolve(t *testing.T) {
now := parseTestTime(t, "2024-08-09T10:32:36Z")
now := parseTestTime(t, "2024-08-16T10:32:36Z")
minTime := parseTestTime(t, "2024-01-01T00:32:36Z")
maxTime := parseTestTime(t, "2024-08-06T06:32:36Z")
watermark := parseTestTime(t, "2024-08-05T06:32:36Z")
testCases := []struct {
Expand All @@ -23,22 +24,30 @@ func Test_Resolve(t *testing.T) {
{`-5m, 0m : |m|`, "2024-08-05T06:27:00Z", "2024-08-05T06:32:00Z"},
{`h : m`, "2024-08-05T06:00:00Z", "2024-08-05T06:33:00Z"},
{`-7d, 0d : |h|`, "2024-07-29T00:00:00Z", "2024-08-05T00:00:00Z"},
{`-7d, now/d : |h|`, "2024-08-02T00:00:00Z", "2024-08-09T00:00:00Z"},
{`-6d, now : |h|`, "2024-08-03T00:00:00Z", "2024-08-09T10:00:00Z"},
{`-6d, now : h`, "2024-08-03T00:00:00Z", "2024-08-09T11:00:00Z"},
{`-7d, now/d : |h|`, "2024-08-09T00:00:00Z", "2024-08-16T00:00:00Z"},
{`-6d, now : |h|`, "2024-08-10T00:00:00Z", "2024-08-16T10:00:00Z"},
{`-6d, now : h`, "2024-08-10T00:00:00Z", "2024-08-16T11:00:00Z"},

{`0Y, now @ {UTC}`, "2024-01-01T00:00:00Z", "2024-08-16T10:32:37Z"},
{`0Y, latest @ {UTC}`, "2024-01-01T00:00:00Z", "2024-08-06T06:32:37Z"},
{`0Y, watermark @ {UTC}`, "2024-01-01T00:00:00Z", "2024-08-05T06:32:37Z"},
{`0y, watermark @ {UTC}`, "2024-01-01T00:00:00Z", "2024-08-05T06:32:37Z"},

{`0d : h`, "2024-08-05T00:00:00Z", "2024-08-05T07:00:00Z"},
{`0d : h @ -1d`, "2024-08-04T00:00:00Z", "2024-08-04T07:00:00Z"},
{`0d : h @ now`, "2024-08-16T00:00:00Z", "2024-08-16T11:00:00Z"},

{`-7d, -5d : h`, "2024-07-29T00:00:00Z", "2024-07-31T00:00:00Z"},
{`-2d, now/d : h @ -5d`, "2024-08-02T00:00:00Z", "2024-08-04T00:00:00Z"},
{`-2d, now/d @ -5d`, "2024-08-02T00:00:00Z", "2024-08-04T00:00:00Z"},
{`watermark-7d, watermark-5d : h`, "2024-07-29T00:00:00Z", "2024-07-31T00:00:00Z"},
{`-2d, now/d : h @ -5d`, "2024-08-09T00:00:00Z", "2024-08-11T00:00:00Z"},
{`-2d, now/d @ -5d`, "2024-08-09T00:00:00Z", "2024-08-11T00:00:00Z"},
{`-7d, -5d @ now`, "2024-08-09T00:00:00Z", "2024-08-11T00:00:00Z"},

{`watermark-7D, watermark : h`, "2024-07-29T07:00:00Z", "2024-08-05T07:00:00Z"},
{`watermark-7D, watermark : h`, "2024-07-29T00:00:00Z", "2024-08-05T07:00:00Z"},

{`-7d, now/d : h @ {Asia/Kathmandu}`, "2024-08-01T18:15:00Z", "2024-08-08T18:15:00Z"},
{`-7d, now/d : |h| @ {Asia/Kathmandu}`, "2024-08-01T18:15:00Z", "2024-08-08T18:15:00Z"},
{`-7d, now/d : |h| @ -5d {Asia/Kathmandu}`, "2024-07-27T18:15:00Z", "2024-08-03T18:15:00Z"},
{`-7d, now/d : h @ {Asia/Kathmandu}`, "2024-08-08T18:15:00Z", "2024-08-15T18:15:00Z"},
{`-7d, now/d : |h| @ {Asia/Kathmandu}`, "2024-08-08T18:15:00Z", "2024-08-15T18:15:00Z"},
{`-7d, now/d : |h| @ -5d {Asia/Kathmandu}`, "2024-08-03T18:15:00Z", "2024-08-10T18:15:00Z"},

{`-7d, latest/d : |h|`, "2024-07-30T00:00:00Z", "2024-08-06T00:00:00Z"},
{`-6d, latest : |h|`, "2024-07-31T00:00:00Z", "2024-08-06T06:00:00Z"},
Expand All @@ -50,8 +59,17 @@ func Test_Resolve(t *testing.T) {
{`2024-01-01 12:00, latest : h`, "2024-01-01T12:00:00Z", "2024-08-06T07:00:00Z"},

{`2024-01-01+5d, latest : h`, "2024-01-06T00:00:00Z", "2024-08-06T07:00:00Z"},
{`-7W+5d, latest : h`, "2024-06-17T00:00:00Z", "2024-08-06T07:00:00Z"},
{`-7W+8d, latest : h`, "2024-06-24T00:00:00Z", "2024-08-06T07:00:00Z"},
{`-7W+5d, latest : h`, "2024-06-22T00:00:00Z", "2024-08-06T07:00:00Z"},
{`-7w+5D, latest : h`, "2024-06-22T00:00:00Z", "2024-08-06T07:00:00Z"},
{`-7W+8d, latest : h`, "2024-06-25T00:00:00Z", "2024-08-06T07:00:00Z"},

{`0W, 0W+1D`, "2024-08-05T00:00:00Z", "2024-08-06T00:00:00Z"},
{`watermark/W, watermark/W+1D`, "2024-08-05T00:00:00Z", "2024-08-06T00:00:00Z"},
{`0W, 0W+1D @ latest`, "2024-08-05T00:00:00Z", "2024-08-06T00:00:00Z"},
{`0W, 0W+1D @ now`, "2024-08-12T00:00:00Z", "2024-08-13T00:00:00Z"},
{`now/W, now/W+1D`, "2024-08-12T00:00:00Z", "2024-08-13T00:00:00Z"},
{`0W, 0W+1D @ 2024-02-08`, "2024-02-05T00:00:00Z", "2024-02-06T00:00:00Z"},
{`0D, 0D+5H @ 2024-02-08 21:34`, "2024-02-08T00:00:00Z", "2024-02-08T05:00:00Z"},

{"P2DT10H", "2024-08-03T20:00:00Z", "2024-08-06T07:32:36Z"},
{"rill-MTD", "2024-08-01T00:00:00Z", "2024-08-06T06:32:37Z"},
Expand All @@ -65,7 +83,7 @@ func Test_Resolve(t *testing.T) {

start, end, err := rillTime.Eval(EvalOptions{
Now: now,
MinTime: now.AddDate(-1, 0, 0),
MinTime: minTime,
MaxTime: maxTime,
Watermark: watermark,
FirstDay: 1,
Expand Down
7 changes: 7 additions & 0 deletions runtime/pkg/timeutil/timeutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ func TruncateTime(start time.Time, tg TimeGrain, tz *time.Location, firstDay, fi
start = start.AddDate(0, -monthsToSubtract, 0)
return start.In(time.UTC)
case TimeGrainYear:
if firstMonth < 1 {
firstMonth = 1
}
if firstMonth > 12 {
firstMonth = 12
}

start = start.In(tz)
year := start.Year()
if int(start.Month()) < firstMonth {
Expand Down
4 changes: 4 additions & 0 deletions runtime/pkg/timeutil/timeutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ func TestTruncateTime_UTC_first_month(t *testing.T) {
require.Equal(t, parseTestTime(t, "2023-03-01T00:00:00Z"), TruncateTime(parseTestTime(t, "2023-03-01T00:20:00Z"), TimeGrainYear, tz, 2, 3))
require.Equal(t, parseTestTime(t, "2022-12-01T00:00:00Z"), TruncateTime(parseTestTime(t, "2023-10-01T00:20:00Z"), TimeGrainYear, tz, 2, 12))
require.Equal(t, parseTestTime(t, "2023-01-01T00:00:00Z"), TruncateTime(parseTestTime(t, "2023-01-01T00:20:00Z"), TimeGrainYear, tz, 2, 1))

// Invalid firstMonth
require.Equal(t, parseTestTime(t, "2023-01-01T00:00:00Z"), TruncateTime(parseTestTime(t, "2023-01-01T00:20:00Z"), TimeGrainYear, tz, 2, 0))
require.Equal(t, parseTestTime(t, "2022-12-01T00:00:00Z"), TruncateTime(parseTestTime(t, "2023-10-01T00:20:00Z"), TimeGrainYear, tz, 2, 13))
}

func TestTruncateTime_Kathmandu_first_month(t *testing.T) {
Expand Down
Loading