Skip to content

Commit

Permalink
Parse ExecutionDuration for SQL queries (#5027)
Browse files Browse the repository at this point in the history
<!-- Describe what has changed in this PR -->
**What changed?**
Support duration expression (eg: "1m", "20h") in SQL queries involving
`ExecutionDuration`.

<!-- Tell your future self why have you made these changes -->
**Why?**
Better UX in advanced visibility with SQL (already supported with ES).
Raw field is in nanoseconds.

<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Created some workflows, and send some queries using those expression.

<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
No.

<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No.
  • Loading branch information
rodrigozhou committed Oct 26, 2023
1 parent 6a82c54 commit 9c14ce1
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 30 deletions.
Expand Up @@ -25,8 +25,6 @@
package elasticsearch

import (
"errors"
"fmt"
"time"

enumspb "go.temporal.io/api/enums/v1"
Expand Down Expand Up @@ -133,14 +131,11 @@ func (vi *valuesInterceptor) Values(name string, values ...interface{}) ([]inter
value = duration.Nanoseconds()
} else {
// To support "hh:mm:ss" durations.
durationNanos, err := vi.parseHHMMSSDuration(durationStr)
var converterErr *query.ConverterError
if errors.As(err, &converterErr) {
return nil, converterErr
}
if err == nil {
value = durationNanos
duration, err := timestamp.ParseHHMMSSDuration(durationStr)
if err != nil {
return nil, err
}
value = duration.Nanoseconds()
}
}
default:
Expand All @@ -150,22 +145,3 @@ func (vi *valuesInterceptor) Values(name string, values ...interface{}) ([]inter
}
return result, nil
}

func (vi *valuesInterceptor) parseHHMMSSDuration(d string) (int64, error) {
var hours, minutes, seconds, nanos int64
_, err := fmt.Sscanf(d, "%d:%d:%d", &hours, &minutes, &seconds)
if err != nil {
return 0, errors.New("value is not a duration")
}
if hours < 0 {
return 0, query.NewConverterError("invalid duration: hours must be positive number")
}
if minutes < 0 || minutes > 59 {
return 0, query.NewConverterError("invalid duration: minutes must be from 0 to 59")
}
if seconds < 0 || seconds > 59 {
return 0, query.NewConverterError("invalid duration: seconds must be from 0 to 59")
}

return hours*int64(time.Hour) + minutes*int64(time.Minute) + seconds*int64(time.Second) + nanos, nil
}
Expand Up @@ -148,12 +148,12 @@ func (s *QueryInterceptorSuite) TestDurationProcessFunc() {
value interface{}
returnErr bool
}{
{value: "1", returnErr: false},
{value: nil, returnErr: true},
{value: 1, returnErr: false},
{value: int64(18180000000000), returnErr: false},
{value: int64(1000000000), returnErr: false},
{value: nil, returnErr: true},
{value: "bad value", returnErr: false},
{value: nil, returnErr: true},
{value: "should not be modified", returnErr: false},
}

Expand Down
22 changes: 22 additions & 0 deletions common/persistence/visibility/store/sql/query_converter.go
Expand Up @@ -37,6 +37,7 @@ import (
"go.temporal.io/server/common/namespace"
"go.temporal.io/server/common/persistence/sql/sqlplugin"
"go.temporal.io/server/common/persistence/visibility/store/query"
"go.temporal.io/server/common/primitives/timestamp"
"go.temporal.io/server/common/searchattribute"
)

Expand Down Expand Up @@ -604,6 +605,27 @@ func (c *QueryConverter) parseSQLVal(
return status, nil
}

if saName == searchattribute.ExecutionDuration {
if durationStr, isString := value.(string); isString {
// To support durations passed as golang durations such as "300ms", "-1.5h" or "2h45m".
// Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
// Custom timestamp.ParseDuration also supports "d" as additional unit for days.
if duration, err := timestamp.ParseDuration(durationStr); err == nil {
value = duration.Nanoseconds()
} else {
// To support "hh:mm:ss" durations.
durationNanos, err := timestamp.ParseHHMMSSDuration(durationStr)
var converterErr *query.ConverterError
if errors.As(err, &converterErr) {
return nil, converterErr
}
if err == nil {
value = durationNanos
}
}
}
}

return value, nil
}

Expand Down
25 changes: 25 additions & 0 deletions common/primitives/timestamp/parse_duration.go
Expand Up @@ -23,6 +23,7 @@
package timestamp

import (
"errors"
"fmt"
"regexp"
"strconv"
Expand All @@ -33,6 +34,11 @@ import (
var (
reUnitless = regexp.MustCompile(`^(\d+(\.\d*)?|(\.\d+))$`)
reDays = regexp.MustCompile(`(\d+(\.\d*)?|(\.\d+))d`)

errInvalidDuration = errors.New("invalid duration")
errInvalidDurationHours = errors.New("invalid duration: hours must be a positive number")
errInvalidDurationMinutes = errors.New("invalid duration: minutes must be from 0 to 59")
errInvalidDurationSeconds = errors.New("invalid duration: seconds must be from 0 to 59")
)

// ParseDuration is like time.ParseDuration, but supports unit "d" for days
Expand Down Expand Up @@ -67,3 +73,22 @@ func ParseDurationDefaultSeconds(s string) (time.Duration, error) {
}
return ParseDuration(s)
}

func ParseHHMMSSDuration(d string) (time.Duration, error) {
var hours, minutes, seconds time.Duration
_, err := fmt.Sscanf(d, "%d:%d:%d", &hours, &minutes, &seconds)
if err != nil {
return 0, errInvalidDuration
}
if hours < 0 {
return 0, errInvalidDurationHours
}
if minutes < 0 || minutes > 59 {
return 0, errInvalidDurationMinutes
}
if seconds < 0 || seconds > 59 {
return 0, errInvalidDurationSeconds
}

return hours*time.Hour + minutes*time.Minute + seconds*time.Second, nil
}
22 changes: 22 additions & 0 deletions common/primitives/timestamp/parse_duration_test.go
Expand Up @@ -102,3 +102,25 @@ func (s *ParseDurationSuite) TestParseDurationDefaultSeconds() {
}
}
}

func (s *ParseDurationSuite) TestParseHHMMSSDuration() {
for _, c := range []struct {
input string
expected time.Duration // -1 means error
}{
{"1:00:00", 1 * time.Hour},
{"123:05:10", 123*time.Hour + 5*time.Minute + 10*time.Second},
{"00:05:10", 5*time.Minute + 10*time.Second},
{"-12:05:10", -1},
{"12:61:10", -1},
{"12:05:61", -1},
{"", -1},
} {
got, err := ParseHHMMSSDuration(c.input)
if c.expected == -1 {
s.Error(err)
} else {
s.Equal(c.expected, got)
}
}
}

0 comments on commit 9c14ce1

Please sign in to comment.