Skip to content

Commit

Permalink
Default anchors to watermark instead of now
Browse files Browse the repository at this point in the history
  • Loading branch information
AdityaHegde committed Jan 20, 2025
1 parent 96a571b commit f480b54
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 84 deletions.
4 changes: 2 additions & 2 deletions runtime/compilers/rillv1/parse_canvas.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (p *Parser) parseCanvas(node *Node) error {
// Build and validate time ranges
var timeRanges []*runtimev1.ExploreTimeRange
for _, tr := range tmp.TimeRanges {
if _, err := rilltime.Parse(tr.Range); err != nil {
if _, err := rilltime.Parse(tr.Range, rilltime.ParseOptions{}); err != nil {
return fmt.Errorf("invalid time range %q: %w", tr.Range, err)
}
res := &runtimev1.ExploreTimeRange{Range: tr.Range}
Expand Down Expand Up @@ -145,7 +145,7 @@ func (p *Parser) parseCanvas(node *Node) error {
var defaultPreset *runtimev1.CanvasPreset
if tmp.Defaults != nil {
if tmp.Defaults.TimeRange != "" {
if _, err := rilltime.Parse(tmp.Defaults.TimeRange); err != nil {
if _, err := rilltime.Parse(tmp.Defaults.TimeRange, rilltime.ParseOptions{}); err != nil {
return fmt.Errorf("invalid time range %q: %w", tmp.Defaults.TimeRange, err)
}
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/compilers/rillv1/parse_explore.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (p *Parser) parseExplore(node *Node) error {
// Build and validate time ranges
var timeRanges []*runtimev1.ExploreTimeRange
for _, tr := range tmp.TimeRanges {
if _, err := rilltime.Parse(tr.Range); err != nil {
if _, err := rilltime.Parse(tr.Range, rilltime.ParseOptions{}); err != nil {
return fmt.Errorf("invalid time range %q: %w", tr.Range, err)
}
res := &runtimev1.ExploreTimeRange{Range: tr.Range}
Expand Down Expand Up @@ -195,7 +195,7 @@ func (p *Parser) parseExplore(node *Node) error {
var defaultPreset *runtimev1.ExplorePreset
if tmp.Defaults != nil {
if tmp.Defaults.TimeRange != "" {
if _, err := rilltime.Parse(tmp.Defaults.TimeRange); err != nil {
if _, err := rilltime.Parse(tmp.Defaults.TimeRange, rilltime.ParseOptions{}); err != nil {
return fmt.Errorf("invalid time range %q: %w", tmp.Defaults.TimeRange, err)
}
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/compilers/rillv1/parse_metrics_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ func (p *Parser) parseMetricsView(node *Node) error {
}

if tmp.DefaultTimeRange != "" {
_, err := rilltime.Parse(tmp.DefaultTimeRange)
_, err := rilltime.Parse(tmp.DefaultTimeRange, rilltime.ParseOptions{})
if err != nil {
return fmt.Errorf(`invalid "default_time_range": %w`, err)
}
Expand Down Expand Up @@ -742,7 +742,7 @@ func (p *Parser) parseMetricsView(node *Node) error {

if tmp.AvailableTimeRanges != nil {
for _, r := range tmp.AvailableTimeRanges {
_, err := rilltime.Parse(r.Range)
_, err := rilltime.Parse(r.Range, rilltime.ParseOptions{})
if err != nil {
return fmt.Errorf("invalid range in available_time_ranges: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/metricsview/executor_rewrite_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (e *Executor) resolveTimeRange(tr *TimeRange, tz *time.Location, minTime, m
return errors.New("other fields are not supported when expression is provided")
}

rillTime, err := rilltime.Parse(tr.Expression)
rillTime, err := rilltime.Parse(tr.Expression, rilltime.ParseOptions{})
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/pkg/metricssql/time_range_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func parseTimeRangeArgs(args []ast.ExprNode) (*rilltime.Expression, error) {
return nil, err
}

rt, err := rilltime.Parse(strings.TrimSuffix(strings.TrimPrefix(du, "'"), "'"))
rt, err := rilltime.Parse(strings.TrimSuffix(strings.TrimPrefix(du, "'"), "'"), rilltime.ParseOptions{})
if err != nil {
return nil, fmt.Errorf("metrics sql: invalid ISO8601 duration %s", du)
}
Expand Down
135 changes: 68 additions & 67 deletions runtime/pkg/rilltime/rilltime.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ type AtModifiers struct {
TimeZone *string `parser:"@TimeZone?"`
}

// ParseOptions allows for additional options that could probably not be added to the time range itself
type ParseOptions struct {
DefaultTimeZone *time.Location
}

type EvalOptions struct {
Now time.Time
MinTime time.Time
Expand All @@ -118,11 +123,11 @@ type EvalOptions struct {
FirstMonth int
}

func Parse(from string) (*Expression, error) {
func Parse(from string, parseOpts ParseOptions) (*Expression, error) {
var rt *Expression
var err error

rt, err = ParseISO(from, false)
rt, err = parseISO(from, parseOpts)
if err != nil {
return nil, err
}
Expand All @@ -136,6 +141,9 @@ func Parse(from string) (*Expression, error) {
}

rt.timeZone = time.UTC
if parseOpts.DefaultTimeZone != nil {
rt.timeZone = parseOpts.DefaultTimeZone
}
if rt.Modifiers != nil {
if rt.Modifiers.Grain != nil {
rt.truncateGrain = grainMap[rt.Modifiers.Grain.Grain]
Expand All @@ -158,64 +166,7 @@ func Parse(from string) (*Expression, error) {

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

return rt, nil
}

func ParseISO(from string, strict bool) (*Expression, error) {
// Try parsing for "inf"
if infPattern.MatchString(from) {
return &Expression{
Start: &TimeAnchor{Earliest: true},
End: &TimeAnchor{Latest: true},
}, nil
}

if strings.HasPrefix(from, "rill-") {
// We are using "rill-" as a prefix to DAX notation so that it doesn't interfere with ISO8601 standard.
// Pulled from https://www.daxpatterns.com/standard-time-related-calculations/
rillDur := strings.Replace(from, "rill-", "", 1)
if t, ok := daxNotations[rillDur]; ok {
return Parse(t)
}
}

// Parse as a regular ISO8601 duration
if !durationPattern.MatchString(from) {
if !strict {
return nil, nil
}
return nil, fmt.Errorf("string %q is not a valid ISO 8601 duration", from)
}

rt := &Expression{
Start: &TimeAnchor{},
End: &TimeAnchor{Latest: true},
// mirrors old UI behaviour
isComplete: false,
}
d, err := duration.ParseISO8601(from)
if err != nil {
if !strict {
return nil, nil
}
return nil, err
}
sd, ok := d.(duration.StandardDuration)
if !ok {
if !strict {
return nil, nil
}
return nil, fmt.Errorf("duration %q is invalid iso format", from)
}
rt.Start.isoDuration = &sd
minGrain := getMinGrain(sd)
if minGrain != "" {
rt.grain = &Grain{
Grain: minGrain,
Watermark: true,
}
}

Expand All @@ -225,7 +176,9 @@ func ParseISO(from string, strict bool) (*Expression, error) {
func ParseCompatibility(timeRange, offset string) error {
isNewFormat := false
if timeRange != "" {
rt, err := Parse(timeRange)
// ParseCompatibility is called for time ranges.
// All parse options should be part of the time range syntax there.
rt, err := Parse(timeRange, ParseOptions{})
if err != nil {
return fmt.Errorf("invalid comparison range %q: %w", timeRange, err)
}
Expand All @@ -243,22 +196,22 @@ func ParseCompatibility(timeRange, offset string) error {
}

func (e *Expression) Eval(evalOpts EvalOptions) (time.Time, time.Time, error) {
start := evalOpts.Now
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.Watermark {
// if end has watermark mentioned then start also should be relative to latest.
start = evalOpts.Watermark
} else if e.End.Now {
// if end has now mentioned then start also should be relative to latest.
start = evalOpts.Now
}
}

if e.Start != nil {
start = e.Modify(evalOpts, e.Start, start, true)
}

end := evalOpts.Now
end := evalOpts.Watermark
if e.End != nil {
end = e.Modify(evalOpts, e.End, end, true)
}
Expand Down Expand Up @@ -328,7 +281,7 @@ func (e *Expression) Modify(evalOpts EvalOptions, ta *TimeAnchor, tm time.Time,
modifiedTime = timeutil.CeilTime(tm, truncateGrain, e.timeZone, evalOpts.FirstDay, evalOpts.FirstMonth)
}

if isBoundary && modifiedTime.Equal(timeBeforeOffset) {
if isBoundary && modifiedTime.Equal(timeBeforeOffset) && (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 @@ -344,6 +297,54 @@ func (e *Expression) Modify(evalOpts EvalOptions, ta *TimeAnchor, tm time.Time,
return modifiedTime
}

func parseISO(from string, parseOpts ParseOptions) (*Expression, error) {
// Try parsing for "inf"
if infPattern.MatchString(from) {
return &Expression{
Start: &TimeAnchor{Earliest: true},
End: &TimeAnchor{Latest: true},
}, nil
}

if strings.HasPrefix(from, "rill-") {
// We are using "rill-" as a prefix to DAX notation so that it doesn't interfere with ISO8601 standard.
// Pulled from https://www.daxpatterns.com/standard-time-related-calculations/
rillDur := strings.Replace(from, "rill-", "", 1)
if t, ok := daxNotations[rillDur]; ok {
return Parse(t, parseOpts)
}
}

// Parse as a regular ISO8601 duration
if !durationPattern.MatchString(from) {
return nil, nil
}

rt := &Expression{
Start: &TimeAnchor{},
End: &TimeAnchor{Latest: true},
// mirrors old UI behaviour
isComplete: false,
}
d, err := duration.ParseISO8601(from)
if err != nil {
return nil, nil
}
sd, ok := d.(duration.StandardDuration)
if !ok {
return nil, nil
}
rt.Start.isoDuration = &sd
minGrain := getMinGrain(sd)
if minGrain != "" {
rt.grain = &Grain{
Grain: minGrain,
}
}

return rt, nil
}

func (g *Grain) offset(tm time.Time) time.Time {
n := 0
if g.Num != nil {
Expand Down
16 changes: 8 additions & 8 deletions runtime/pkg/rilltime/rilltime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ func Test_Resolve(t *testing.T) {
end string
}{
// Earliest = 2023-08-09T10:32:36Z, Latest = 2024-08-06T06:32:36Z, = Now = 2024-08-09T10:32:36Z
{`m : |s|`, "2024-08-09T10:32:00Z", "2024-08-09T10:32:37Z"},
{`m : s`, "2024-08-09T10:32:00Z", "2024-08-09T10:32:37Z"},
{`-5m : |m|`, "2024-08-09T10:27:00Z", "2024-08-09T10:32:00Z"},
{`-5m, 0m : |m|`, "2024-08-09T10:27:00Z", "2024-08-09T10:32:00Z"},
{`h : m`, "2024-08-09T10:00:00Z", "2024-08-09T10:33:00Z"},
{`-7d, 0d : |h|`, "2024-08-02T00:00:00Z", "2024-08-09T00:00:00Z"},
{`m : |s|`, "2024-08-05T06:32:00Z", "2024-08-05T06:32:36Z"},
{`m : s`, "2024-08-05T06:32:00Z", "2024-08-05T06:32:37Z"},
{`-5m : |m|`, "2024-08-05T06:27:00Z", "2024-08-05T06:32:00Z"},
{`-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, -5d : h`, "2024-08-02T00:00:00Z", "2024-08-04T00: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"},

Expand Down Expand Up @@ -57,7 +57,7 @@ func Test_Resolve(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.timeRange, func(t *testing.T) {
rillTime, err := Parse(tc.timeRange)
rillTime, err := Parse(tc.timeRange, ParseOptions{})
require.NoError(t, err)

start, end, err := rillTime.Eval(EvalOptions{
Expand Down
2 changes: 1 addition & 1 deletion runtime/queries/metricsview_resolve_time_ranges.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (q *MetricsViewTimeRanges) Resolve(ctx context.Context, rt *runtime.Runtime

timeRanges := make([]*runtimev1.TimeRange, len(q.Expressions))
for i, tr := range q.Expressions {
rillTime, err := rilltime.Parse(tr)
rillTime, err := rilltime.Parse(tr, rilltime.ParseOptions{})
if err != nil {
return fmt.Errorf("error parsing time range %s: %w", tr, err)
}
Expand Down

0 comments on commit f480b54

Please sign in to comment.