diff --git a/runtime/metricsview/executor/executor_rewrite_time.go b/runtime/metricsview/executor/executor_rewrite_time.go index aa12ba31b0c..8135a401795 100644 --- a/runtime/metricsview/executor/executor_rewrite_time.go +++ b/runtime/metricsview/executor/executor_rewrite_time.go @@ -7,7 +7,6 @@ import ( "time" "github.com/rilldata/rill/runtime/metricsview" - "github.com/rilldata/rill/runtime/pkg/duration" "github.com/rilldata/rill/runtime/pkg/rilltime" "github.com/rilldata/rill/runtime/pkg/timeutil" ) @@ -65,10 +64,17 @@ func (e *Executor) resolveTimeRange(ctx context.Context, tr *metricsview.TimeRan return nil } - if tr.Expression == "" { - return e.resolveISOTimeRange(ctx, tr, tz, executionTime) + // Start and End are hardcoded, skip the rest of the code + if !tr.Start.IsZero() && !tr.End.IsZero() { + // Clear all other fields than Start and End + tr.Expression = "" + tr.IsoDuration = "" + tr.IsoOffset = "" + tr.RoundToGrain = metricsview.TimeGrainUnspecified + return nil } - if !tr.Start.IsZero() || !tr.End.IsZero() || tr.IsoDuration != "" || tr.IsoOffset != "" || tr.RoundToGrain != metricsview.TimeGrainUnspecified { + + if tr.Expression != "" && (!tr.Start.IsZero() || !tr.End.IsZero() || tr.IsoDuration != "" || tr.IsoOffset != "" || tr.RoundToGrain != metricsview.TimeGrainUnspecified) { return errors.New("other fields are not supported when expression is provided") } @@ -84,16 +90,24 @@ func (e *Executor) resolveTimeRange(ctx context.Context, tr *metricsview.TimeRan ts.Now = *executionTime } - rillTime, err := rilltime.Parse(tr.Expression, rilltime.ParseOptions{ - SmallestGrain: timeutil.TimeGrainFromAPI(e.metricsView.SmallestTimeGrain), - DefaultTimeZone: tz, - }) + var rt *rilltime.Expression + if tr.IsoDuration != "" || tr.IsoOffset != "" || tr.RoundToGrain != metricsview.TimeGrainUnspecified { + rt, err = rilltime.ParseISO(tr.IsoDuration, tr.IsoOffset, tr.End, tr.RoundToGrain.ToTimeutil(), rilltime.ParseOptions{ + DefaultTimeZone: tz, + SmallestGrain: timeutil.TimeGrainFromAPI(e.metricsView.SmallestTimeGrain), + }) + } else { + rt, err = rilltime.Parse(tr.Expression, rilltime.ParseOptions{ + DefaultTimeZone: tz, + SmallestGrain: timeutil.TimeGrainFromAPI(e.metricsView.SmallestTimeGrain), + }) + } if err != nil { return err } // TODO: use grain when we have timeseries from metrics_view_aggregation - tr.Start, tr.End, _ = rillTime.Eval(rilltime.EvalOptions{ + tr.Start, tr.End, _ = rt.Eval(rilltime.EvalOptions{ Now: ts.Now, MinTime: ts.Min, MaxTime: ts.Max, @@ -110,86 +124,3 @@ func (e *Executor) resolveTimeRange(ctx context.Context, tr *metricsview.TimeRan return nil } - -// resolveISOTimeRange resolves the given time range where either only start/end is specified along with ISO duration/offset, ensuring only its Start and End properties are populated. -func (e *Executor) resolveISOTimeRange(ctx context.Context, tr *metricsview.TimeRange, tz *time.Location, executionTime *time.Time) error { - if tr.Start.IsZero() && tr.End.IsZero() { - if executionTime == nil { - ts, err := e.Timestamps(ctx, tr.TimeDimension) - if err != nil { - return fmt.Errorf("failed to fetch timestamps: %w", err) - } - executionTime = &ts.Watermark - } - - tr.End = *executionTime - } - - var isISO bool - if tr.IsoDuration != "" { - d, err := duration.ParseISO8601(tr.IsoDuration) - if err != nil { - return fmt.Errorf("invalid iso_duration %q: %w", tr.IsoDuration, err) - } - - if !tr.Start.IsZero() && !tr.End.IsZero() { - return errors.New(`cannot resolve "iso_duration" for a time range with fixed "start" and "end" timestamps`) - } else if !tr.Start.IsZero() { - tr.End = d.Add(tr.Start) - } else if !tr.End.IsZero() { - tr.Start = d.Sub(tr.End) - } else { - // In practice, this shouldn't happen since we resolve a time anchor dynamically if both start and end are zero. - return errors.New(`cannot resolve "iso_duration" for a time range without "start" and "end" timestamps`) - } - - isISO = true - } - - if tr.IsoOffset != "" { - d, err := duration.ParseISO8601(tr.IsoOffset) - if err != nil { - return fmt.Errorf("invalid iso_offset %q: %w", tr.IsoOffset, err) - } - - if !tr.Start.IsZero() { - tr.Start = d.Sub(tr.Start) - } - if !tr.End.IsZero() { - tr.End = d.Sub(tr.End) - } - - isISO = true - } - - // Only modify the start and end if ISO duration or offset was sent. - // This is to maintain backwards compatibility for calls from the UI. - if isISO { - fdow := int(e.metricsView.FirstDayOfWeek) - if fdow > 7 || fdow <= 0 { - fdow = 1 - } - fmoy := int(e.metricsView.FirstMonthOfYear) - if fmoy > 12 || fmoy <= 0 { - fmoy = 1 - } - if !tr.RoundToGrain.Valid() { - return fmt.Errorf("invalid time grain %q", tr.RoundToGrain) - } - if tr.RoundToGrain != metricsview.TimeGrainUnspecified { - if !tr.Start.IsZero() { - tr.Start = timeutil.TruncateTime(tr.Start, tr.RoundToGrain.ToTimeutil(), tz, fdow, fmoy) - } - if !tr.End.IsZero() { - tr.End = timeutil.TruncateTime(tr.End, tr.RoundToGrain.ToTimeutil(), tz, fdow, fmoy) - } - } - } - - // Clear all other fields than Start and End - tr.IsoDuration = "" - tr.IsoOffset = "" - tr.RoundToGrain = metricsview.TimeGrainUnspecified - - return nil -} diff --git a/runtime/pkg/rilltime/rilltime.go b/runtime/pkg/rilltime/rilltime.go index ec2b4f2c2c5..07a664318bf 100644 --- a/runtime/pkg/rilltime/rilltime.go +++ b/runtime/pkg/rilltime/rilltime.go @@ -14,10 +14,11 @@ import ( ) var ( - infPattern = regexp.MustCompile("^(?i)inf$") - durationPattern = regexp.MustCompile(`^P((?P\d+)Y)?((?P\d+)M)?((?P\d+)W)?((?P\d+)D)?(T((?P\d+)H)?((?P\d+)M)?((?P\d+)S)?)?$`) - isoTimePattern = `(?P\d{4})(-(?P\d{2})(-(?P\d{2})(T(?P\d{2})(:(?P\d{2})(:(?P\d{2})(\.((?P\d{3})|(?P\d{6})|(?P\d{9})))?Z)?)?)?)?)?` - isoTimeRegex = regexp.MustCompile(isoTimePattern) + infPattern = regexp.MustCompile("^(?i)inf$") + iso8601DurationPattern = `(?i)P((\d+[YMWD])+(T(\d+[HMS])+)?|T(\d+[HMS])+)` // Ensures atleast one part is present + iso8601PartsRegex = regexp.MustCompile(`(?i)(\d+)([YMWDHS])`) + isoTimePattern = `(?P\d{4})(-(?P\d{2})(-(?P\d{2})(T(?P\d{2})(:(?P\d{2})(:(?P\d{2})(\.((?P\d{3})|(?P\d{6})|(?P\d{9})))?Z)?)?)?)?)?` + isoTimeRegex = regexp.MustCompile(isoTimePattern) // nolint:govet // This is suggested usage by the docs. rillTimeLexer = lexer.MustSimple([]lexer.SimpleRule{ {"Ref", "ref"}, @@ -25,6 +26,7 @@ var ( {"Now", "now"}, {"Latest", "latest"}, {"Watermark", "watermark"}, + {"ISO8601Duration", iso8601DurationPattern}, {"PreviousPeriod", "(?i)p"}, {"Offset", `(?i)offset`}, // this needs to be after Now and Latest to match to them @@ -138,6 +140,7 @@ type Interval struct { PeriodToGrain *PeriodToGrainInterval `parser:"| @@"` StartEnd *StartEndInterval `parser:"| @@"` Ordinal *OrdinalInterval `parser:"| @@"` + LegacyIso *LegacyIsoInterval `parser:"| @@"` Iso *IsoInterval `parser:"| @@)"` } @@ -166,8 +169,13 @@ type StartEndInterval struct { // IsoInterval is an interval formed by ISO timestamps. Allows for partial timestamps in ISOPointInTime. type IsoInterval struct { - Start *ISOPointInTime `parser:"@@"` - End *ISOPointInTime `parser:"((To | '/' | RangeSeparator) @@)?"` + Start *AbsISOPointInTime `parser:"@@"` + End *AbsISOPointInTime `parser:"((To | '/' | RangeSeparator) @@)?"` +} + +type LegacyIsoInterval struct { + ISO string `parser:"@ISO8601Duration"` + Snap *string // Not supported in parsing yet } type PointInTime struct { @@ -177,7 +185,7 @@ type PointInTime struct { type PointInTimeWithSnap struct { Grain *GrainPointInTime `parser:"( @@"` Labeled *LabeledPointInTime `parser:"| @@"` - ISO *ISOPointInTime `parser:"| @@)"` + AbsISO *AbsISOPointInTime `parser:"| @@)"` Snap *string `parser:"(Snap @Grain"` // A secondary snap after the above snap. This allows specifying a time range bucketed by week but snapped by a higher order grain. @@ -203,7 +211,7 @@ type LabeledPointInTime struct { Watermark bool `parser:"| @Watermark)"` } -type ISOPointInTime struct { +type AbsISOPointInTime struct { ISO string `parser:"@ISOTime"` year int @@ -276,7 +284,7 @@ func Parse(from string, parseOpts ParseOptions) (*Expression, error) { if err != nil { return nil, err } - rt.isNewFormat = true + rt.isNewFormat = rt.Interval.LegacyIso == nil if rt.Interval != nil { err := rt.Interval.parse() @@ -331,6 +339,65 @@ func ParseCompatibility(timeRange, offset string) error { return nil } +func ParseISO(duration, offset string, end time.Time, snap timeutil.TimeGrain, parseOpts ParseOptions) (*Expression, error) { + rt, err := parseISO(duration, parseOpts) + if err != nil { + return nil, err + } + + if rt == nil { + return nil, fmt.Errorf("invalid ISO duration %q: %w", duration, err) + } + + if offset != "" { + if offset == "inf" || strings.HasPrefix(offset, "rill-") { + return nil, fmt.Errorf("invalid ISO offset %q: cannot have non-iso for offset", duration) + } + + oi := &LegacyIsoInterval{ISO: offset} + offsetPoint, _, err := oi.toPointInTime() + if err != nil { + return nil, err + } + + rt.AnchorOverrides = append(rt.AnchorOverrides, &PointInTime{ + Points: []*PointInTimeWithSnap{offsetPoint}, + }) + } + + if !end.IsZero() { + ei := &AbsISOPointInTime{ISO: end.UTC().Format(time.RFC3339)} + err = ei.parse() + if err != nil { + return nil, err + } + + pt := &PointInTimeWithSnap{AbsISO: ei} + if snap != timeutil.TimeGrainUnspecified { + s := reverseGrainMap[snap] + pt.Snap = &s + } + + rt.AnchorOverrides = append(rt.AnchorOverrides, &PointInTime{ + Points: []*PointInTimeWithSnap{pt}, + }) + } else { + pt := &PointInTimeWithSnap{ + Labeled: &LabeledPointInTime{Watermark: true}, + } + if snap != timeutil.TimeGrainUnspecified { + s := reverseGrainMap[snap] + pt.Snap = &s + } + + rt.AnchorOverrides = append(rt.AnchorOverrides, &PointInTime{ + Points: []*PointInTimeWithSnap{pt}, + }) + } + + return rt, nil +} + func (e *Expression) Eval(evalOpts EvalOptions) (time.Time, time.Time, timeutil.TimeGrain) { if evalOpts.FirstDay == 0 { evalOpts.FirstDay = 1 @@ -396,6 +463,10 @@ func (i *Interval) parse() error { } else if i.PeriodToGrain != nil { // Period-to-date syntax maps to StartEndInterval as well. i.StartEnd = i.PeriodToGrain.expand() + } else if i.LegacyIso != nil { + var err error + i.StartEnd, err = i.LegacyIso.expand() + return err } else if i.Iso != nil { return i.Iso.parse() } @@ -520,6 +591,104 @@ func (o *StartEndInterval) eval(evalOpts EvalOptions, tm time.Time, tz *time.Loc return start, end, tg } +func (l *LegacyIsoInterval) expand() (*StartEndInterval, error) { + isoPointInTime, offsetPoint, err := l.toPointInTime() + if err != nil { + return nil, err + } + + start := &PointInTime{ + Points: []*PointInTimeWithSnap{ + { + Labeled: &LabeledPointInTime{Watermark: true}, + }, + isoPointInTime, + }, + } + end := &PointInTime{ + Points: []*PointInTimeWithSnap{ + { + Labeled: &LabeledPointInTime{Watermark: true}, + }, + }, + } + + if offsetPoint != nil { + start.Points = append(start.Points, offsetPoint) + end.Points = append(end.Points, offsetPoint) + } + + return &StartEndInterval{ + Start: start, + End: end, + }, nil +} + +func (l *LegacyIsoInterval) toPointInTime() (*PointInTimeWithSnap, *PointInTimeWithSnap, error) { + matches := iso8601PartsRegex.FindAllStringSubmatchIndex(l.ISO, -1) + parts := make([]*GrainDurationPart, len(matches)) + + timePartIndex := strings.Index(l.ISO, "T") + // Set the index to the end if "T" doesnt exist. This simplifies the check inside the loop. + if timePartIndex == -1 { + timePartIndex = len(l.ISO) + } + + for i, match := range matches { + if len(match) != 6 { + return nil, nil, fmt.Errorf("invalid ISO duration %q", l.ISO) + } + numStr := l.ISO[match[2]:match[3]] + grain := l.ISO[match[4]:match[5]] + + if match[4] > timePartIndex { + grain = strings.ToLower(grain) + } + + num, err := strconv.Atoi(numStr) + if err != nil { + return nil, nil, err + } + + parts[i] = &GrainDurationPart{ + Num: num, + Grain: grain, + } + } + mainPoint := &PointInTimeWithSnap{ + Grain: &GrainPointInTime{ + Parts: []*GrainPointInTimePart{ + { + Prefix: "-", + Duration: &GrainDuration{Parts: parts}, + }, + }, + }, + } + + var offsetPoint *PointInTimeWithSnap + if l.Snap != nil { + // ref-iso+1grain/grain to ref+1grain/grain + offsetPoint = &PointInTimeWithSnap{ + Grain: &GrainPointInTime{ + Parts: []*GrainPointInTimePart{ + { + Prefix: "+", + Duration: &GrainDuration{ + Parts: []*GrainDurationPart{ + {Grain: *l.Snap, Num: 1}, + }, + }, + }, + }, + }, + Snap: l.Snap, + } + } + + return mainPoint, offsetPoint, nil +} + func (i *IsoInterval) parse() error { err := i.Start.parse() if err != nil { @@ -581,8 +750,8 @@ func (p *PointInTime) truncates() bool { } func (p *PointInTimeWithSnap) parse() error { - if p.ISO != nil { - return p.ISO.parse() + if p.AbsISO != nil { + return p.AbsISO.parse() } return nil } @@ -593,8 +762,8 @@ func (p *PointInTimeWithSnap) eval(evalOpts EvalOptions, tm time.Time, tz *time. tm, tg = p.Grain.eval(tm, tz) } else if p.Labeled != nil { tm = p.Labeled.eval(evalOpts) - } else if p.ISO != nil { - tm, _, tg = p.ISO.eval(tz) + } else if p.AbsISO != nil { + tm, _, tg = p.AbsISO.eval(tz) } if p.Snap != nil { @@ -651,8 +820,7 @@ func (l *LabeledPointInTime) eval(evalOpts EvalOptions) time.Time { return time.Time{} } -// TODO: reuse code from duration.ParseISO8601 -func (a *ISOPointInTime) parse() error { +func (a *AbsISOPointInTime) parse() error { match := isoTimeRegex.FindStringSubmatch(a.ISO) for i, name := range isoTimeRegex.SubexpNames() { @@ -704,7 +872,7 @@ func (a *ISOPointInTime) parse() error { return nil } -func (a *ISOPointInTime) eval(tz *time.Location) (time.Time, time.Time, timeutil.TimeGrain) { +func (a *AbsISOPointInTime) eval(tz *time.Location) (time.Time, time.Time, timeutil.TimeGrain) { // Since we use this to build a time, month and day cannot be zero, hence the max(1, xx) absStart := time.Date(a.year, time.Month(max(1, a.month)), max(1, a.day), a.hour, a.minute, a.second, a.nano, tz) absEnd := timeutil.OffsetTime(absStart, a.tg, 1, tz) @@ -810,46 +978,7 @@ func parseISO(from string, parseOpts ParseOptions) (*Expression, error) { } } - // Parse as a regular ISO8601 duration - if !durationPattern.MatchString(from) { - return nil, nil - } - - rt := &Expression{} - d, err := duration.ParseISO8601(from) - if err != nil { - return nil, nil - } - sd, ok := d.(duration.StandardDuration) - if !ok { - return nil, nil - } - rt.isoDuration = &sd - minGrain := getMinGrain(sd) - if minGrain != "" { - rt.Grain = &minGrain - } - - return rt, nil -} - -func getMinGrain(d duration.StandardDuration) string { - if d.Second != 0 { - return "s" - } else if d.Minute != 0 { - return "m" - } else if d.Hour != 0 { - return "h" - } else if d.Day != 0 { - return "D" - } else if d.Week != 0 { - return "W" - } else if d.Month != 0 { - return "M" - } else if d.Year != 0 { - return "Y" - } - return "" + return nil, nil } // truncateWithCorrection truncates time by a grain but corrects for https://en.wikipedia.org/wiki/ISO_week_date#First_week diff --git a/runtime/pkg/rilltime/rilltime_test.go b/runtime/pkg/rilltime/rilltime_test.go index 656c8795568..f2718d67aae 100644 --- a/runtime/pkg/rilltime/rilltime_test.go +++ b/runtime/pkg/rilltime/rilltime_test.go @@ -1,6 +1,7 @@ package rilltime import ( + "fmt" "testing" "time" @@ -420,7 +421,7 @@ func TestEval_BackwardsCompatibility(t *testing.T) { // `inf` => `earliest to latest+1s` {"inf", "2020-01-01T00:32:36Z", "2025-05-14T06:32:37Z", timeutil.TimeGrainUnspecified, 1, 1}, - {"P2DT10H", "2025-05-10T21:00:00Z", "2025-05-13T07:00:00Z", timeutil.TimeGrainHour, 1, 1}, + {"P1Y2M3W4DT10H15M", "2024-02-17T20:17:36Z", "2025-05-13T06:32:36Z", timeutil.TimeGrainDay, 1, 1}, } runTests(t, testCases, now, minTime, maxTime, watermark, nil) @@ -510,6 +511,7 @@ func runTests(t *testing.T, testCases []testCase, now, minTime, maxTime, waterma FirstDay: testCase.FirstDay, FirstMonth: testCase.FirstMonth, }) + fmt.Println(start, end) require.Equal(t, parseTestTime(t, testCase.start), start) require.Equal(t, parseTestTime(t, testCase.end), end) if testCase.grain != timeutil.TimeGrainUnspecified {