Simplify/centralize time parsing

We had a bunch of different places which had basically the
same logic, except that some were testing both RFC3339Nano
and RFC3339 formats, and some weren't.

This turns out not to matter, because the fractional second
part is always permitted and never required, so those two
formats are identical.

Mostly, though, we now ensure that everything we do that is
trying to convert timestamps has the same logic, so if we
want to make changes to that logic, we have a central point,
which lives in the parser.

This came out of an attempt to figure out why the RFC3339
case wasn't getting any test coverage.
This commit is contained in:
Seebs 2023-04-07 17:02:37 -05:00
parent 6787a8ec58
commit a158bba125
10 changed files with 52 additions and 70 deletions

View file

@ -1838,16 +1838,21 @@ func (expr *StringLit) Pos() Pos {
}
func (expr *StringLit) ConvertToTimestamp() *DateLit {
//try to coerce to a date
if tm, err := time.ParseInLocation(time.RFC3339Nano, expr.Value, time.UTC); err == nil {
return &DateLit{ValuePos: expr.ValuePos, Value: tm}
} else if tm, err := time.ParseInLocation(time.RFC3339, expr.Value, time.UTC); err == nil {
return &DateLit{ValuePos: expr.ValuePos, Value: tm}
} else if tm, err := time.ParseInLocation("2006-01-02", expr.Value, time.UTC); err == nil {
return &DateLit{ValuePos: expr.ValuePos, Value: tm}
} else {
tm, err := ConvertStringToTimestamp(expr.Value)
if err != nil {
return nil
}
return &DateLit{ValuePos: expr.ValuePos, Value: tm}
}
func ConvertStringToTimestamp(date string) (time.Time, error) {
if tm, err := time.ParseInLocation(time.RFC3339, date, time.UTC); err == nil {
return tm, nil
} else if tm, err := time.ParseInLocation("2006-01-02", date, time.UTC); err == nil {
return tm, nil
} else {
return time.Time{}, err
}
}
// Clone returns a deep copy of lit.

View file

@ -89,13 +89,11 @@ func coerceValue(sourceType parser.ExprDataType, targetType parser.ExprDataType,
if !ok {
return nil, sql3.NewErrInternalf("unexpected value type '%T'", value)
}
if tm, err := time.ParseInLocation(time.RFC3339Nano, val, time.UTC); err == nil {
return tm, nil
} else if tm, err := time.ParseInLocation("2006-01-02", val, time.UTC); err == nil {
return tm, nil
} else {
tm, err := parser.ConvertStringToTimestamp(val)
if err != nil {
return nil, sql3.NewErrInvalidTypeCoercion(0, 0, val, targetType.TypeDescription())
}
return tm, nil
}
case *parser.DataTypeTimestamp:
@ -2140,16 +2138,11 @@ func (n *stringLiteralPlanExpression) WithChildren(children ...types.PlanExpress
}
func (expr *stringLiteralPlanExpression) ConvertToTimestamp() *time.Time {
// try to coerce to a date
if tm, err := time.ParseInLocation(time.RFC3339Nano, expr.value, time.UTC); err == nil {
return &tm
} else if tm, err := time.ParseInLocation(time.RFC3339, expr.value, time.UTC); err == nil {
return &tm
} else if tm, err := time.ParseInLocation("2006-01-02", expr.value, time.UTC); err == nil {
return &tm
} else {
tm, err := parser.ConvertStringToTimestamp(expr.value)
if err != nil {
return nil
}
return &tm
}
// castPlanExpressionis a cast op
@ -2289,15 +2282,11 @@ func (n *castPlanExpression) Evaluate(currentRow []interface{}) (interface{}, er
return nl, nil
case *parser.DataTypeTimestamp:
if tm, err := time.ParseInLocation(time.RFC3339Nano, nl, time.UTC); err == nil {
return tm, nil
} else if tm, err := time.ParseInLocation(time.RFC3339, nl, time.UTC); err == nil {
return tm, nil
} else if tm, err := time.ParseInLocation("2006-01-02", nl, time.UTC); err == nil {
return tm, nil
} else {
tm, err := parser.ConvertStringToTimestamp(nl)
if err != nil {
return nil, sql3.NewErrInvalidCast(0, 0, nl, n.targetType.TypeDescription())
}
return tm, nil
}
case *parser.DataTypeStringSet:
@ -2999,15 +2988,3 @@ func wildCardToRegexp(pattern string) string {
return result.String()
}
// timeFromString attempts to parse the string to a time.Time using a series of
// time formats.
func timestampFromString(s string) (time.Time, error) {
if tm, err := time.ParseInLocation(time.RFC3339Nano, s, time.UTC); err == nil {
return tm, nil
} else if tm, err := time.ParseInLocation("2006-01-02", s, time.UTC); err == nil {
return tm, nil
}
return time.Time{}, sql3.NewErrInvalidTypeCoercion(0, 0, s, "time.Time")
}

View file

@ -2,7 +2,6 @@ package planner
import (
"testing"
"time"
"github.com/featurebasedb/featurebase/v3/sql3/parser"
"github.com/featurebasedb/featurebase/v3/sql3/planner/types"
@ -64,7 +63,7 @@ func TestExpressions(t *testing.T) {
blop := newBoolLiteralPlanExpression(false)
assert.Equal(t, blop.String(), "false")
tm, _ := time.ParseInLocation(time.RFC3339, "2012-11-01T22:08:41+00:00", time.UTC)
tm, _ := parser.ConvertStringToTimestamp("2012-11-01T22:08:41+00:00")
dlop := newTimestampLiteralPlanExpression(tm)
assert.Equal(t, dlop.String(), "2012-11-01T22:08:41Z")

View file

@ -317,13 +317,11 @@ func (i *bulkInsertSourceCSVRowIter) Next(ctx context.Context) (types.Row, error
case *parser.DataTypeTimestamp:
intVal, err := strconv.ParseInt(evalValue, 10, 64)
if err != nil {
if tm, err := time.ParseInLocation(time.RFC3339Nano, evalValue, time.UTC); err == nil {
result[idx] = tm
} else if tm, err := time.ParseInLocation("2006-01-02", evalValue, time.UTC); err == nil {
result[idx] = tm
} else {
tm, err := parser.ConvertStringToTimestamp(evalValue)
if err != nil {
return nil, sql3.NewErrTypeConversionOnMap(0, 0, evalValue, mapColumn.colType.TypeDescription())
}
result[idx] = tm
} else {
// implicit conversion of int to timestamp will treat int as seconds since unix epoch
result[idx] = time.Unix(intVal, 0).UTC()
@ -661,13 +659,11 @@ func (i *bulkInsertSourceNDJsonRowIter) Next(ctx context.Context) (types.Row, er
return nil, sql3.NewErrTypeConversionOnMap(0, 0, v, mapColumn.colType.TypeDescription())
case string:
if tm, err := time.ParseInLocation(time.RFC3339Nano, v, time.UTC); err == nil {
result[idx] = tm
} else if tm, err := time.ParseInLocation("2006-01-02", v, time.UTC); err == nil {
result[idx] = tm
} else {
tm, err := parser.ConvertStringToTimestamp(v)
if err != nil {
return nil, sql3.NewErrTypeConversionOnMap(0, 0, v, mapColumn.colType.TypeDescription())
}
result[idx] = tm
case bool:
return nil, sql3.NewErrTypeConversionOnMap(0, 0, v, mapColumn.colType.TypeDescription())
@ -1153,13 +1149,11 @@ func (i *bulkInsertSourceParquetRowIter) Next(ctx context.Context) (types.Row, e
// implicit conversion of int to timestamp will treat int as seconds since unix epoch
result[idx] = time.Unix(intVal, 0).UTC()
} else if stringVal, ok := evalValue.(string); ok {
if tm, err := time.ParseInLocation(time.RFC3339Nano, stringVal, time.UTC); err == nil {
result[idx] = tm
} else if tm, err := time.ParseInLocation("2006-01-02", stringVal, time.UTC); err == nil {
result[idx] = tm
} else {
tm, err := parser.ConvertStringToTimestamp(stringVal)
if err != nil {
return nil, sql3.NewErrTypeConversionOnMap(0, 0, stringVal, mapColumn.colType.TypeDescription())
}
result[idx] = tm
}
case *parser.DataTypeString:

View file

@ -375,7 +375,7 @@ func (i *insertRowIter) Next(ctx context.Context) (types.Row, error) {
// string is the normal case for dates; used when the date is
// provided as a string in the INSERT INTO statement.
case string:
ts, err := timestampFromString(v)
ts, err := parser.ConvertStringToTimestamp(v)
if err != nil {
return nil, errors.Wrapf(err, "parsing timestamp: %s", v)
}

View file

@ -6,7 +6,6 @@ import (
"context"
"fmt"
"strings"
"time"
pilosa "github.com/featurebasedb/featurebase/v3"
"github.com/featurebasedb/featurebase/v3/dax"
@ -230,7 +229,7 @@ func (i *distinctScanRowIter) Next(ctx context.Context) (types.Row, error) {
case pilosa.DistinctTimestamp:
result := make([]interface{}, 0)
for _, n := range res.Values {
if tm, err := time.ParseInLocation(time.RFC3339Nano, n, time.UTC); err == nil {
if tm, err := parser.ConvertStringToTimestamp(n); err == nil {
result = append(result, tm)
} else {
return nil, sql3.NewErrInternalf("unable to convert to time.Time: %v", n)

View file

@ -21,6 +21,7 @@ import (
pilosa "github.com/featurebasedb/featurebase/v3"
"github.com/featurebasedb/featurebase/v3/dax"
"github.com/featurebasedb/featurebase/v3/pql"
"github.com/featurebasedb/featurebase/v3/sql3/parser"
sql_test "github.com/featurebasedb/featurebase/v3/sql3/test"
"github.com/featurebasedb/featurebase/v3/test"
"github.com/featurebasedb/featurebase/v3/vprint"
@ -3055,9 +3056,9 @@ func TestPlanner_BulkInsertParquet(t *testing.T) {
// order by timestamp
results, _, _, err = sql_test.MustQueryRows(t, nil, c.GetNode(0).Server, `select _id, t from j1 order by t`)
assert.NoError(t, err)
t2, _ := time.ParseInLocation(time.RFC3339Nano, "1970-01-28T00:00:00Z", time.UTC)
t3, _ := time.ParseInLocation(time.RFC3339Nano, "1988-05-30T12:02:00Z", time.UTC)
t1, _ := time.ParseInLocation(time.RFC3339Nano, "2022-01-28T12:14:04Z", time.UTC)
t2, _ := parser.ConvertStringToTimestamp("1970-01-28T00:00:00Z")
t3, _ := parser.ConvertStringToTimestamp("1988-05-30T12:02:00Z")
t1, _ := parser.ConvertStringToTimestamp("2022-01-28T12:14:04Z")
if diff := cmp.Diff([][]interface{}{
{int64(2), t2},

View file

@ -11,6 +11,7 @@ import (
"github.com/PaesslerAG/gval"
"github.com/PaesslerAG/jsonpath"
"github.com/featurebasedb/featurebase/v3/errors"
"github.com/featurebasedb/featurebase/v3/sql3/parser"
)
// TableTests is the list of tests which get run by TestSQL_Execute in
@ -219,7 +220,7 @@ var TableTests []TableTest = []TableTest{
}
func knownTimestamp() time.Time {
tm, err := time.ParseInLocation(time.RFC3339, "2012-11-01T22:08:41+00:00", time.UTC)
tm, err := parser.ConvertStringToTimestamp("2012-11-01T22:08:41+00:00")
if err != nil {
panic(err.Error())
}
@ -237,7 +238,7 @@ func knownSubSecondTimestamp() time.Time {
}
func knownSubSecondTimestamp2() time.Time {
tm, err := time.ParseInLocation(time.RFC3339, "2022-12-09T18:04:54+00:00", time.UTC)
tm, err := parser.ConvertStringToTimestamp("2022-12-09T18:04:54+00:00")
if err != nil {
panic(err.Error())
}
@ -250,7 +251,7 @@ func knownSubSecondTimestamp2() time.Time {
}
func timestampFromString(s string) time.Time {
tm, err := time.ParseInLocation(time.RFC3339, s, time.UTC)
tm, err := parser.ConvertStringToTimestamp(s)
if err != nil {
panic(err.Error())
}

View file

@ -1,10 +1,14 @@
// Copyright 2021 Molecula Corp. All rights reserved.
package defs
import "time"
import (
"time"
"github.com/featurebasedb/featurebase/v3/sql3/parser"
)
func earlyMay2022() time.Time {
tm, err := time.ParseInLocation(time.RFC3339, "2022-05-05T13:00:00+00:00", time.UTC)
tm, err := parser.ConvertStringToTimestamp("2022-05-05T13:00:00+00:00")
if err != nil {
panic(err.Error())
}
@ -12,7 +16,7 @@ func earlyMay2022() time.Time {
}
func lateMay2022() time.Time {
tm, err := time.ParseInLocation(time.RFC3339, "2022-05-28T13:00:00+00:00", time.UTC)
tm, err := parser.ConvertStringToTimestamp("2022-05-28T13:00:00+00:00")
if err != nil {
panic(err.Error())
}

View file

@ -3,6 +3,8 @@ package defs
import (
"fmt"
"time"
"github.com/featurebasedb/featurebase/v3/sql3/parser"
)
var sql1TestsGrouper = TableTest{
@ -74,7 +76,7 @@ var sql1TestsDelete = TableTest{
// grouperTimeX extracts the time associated with record ID x.
// note that the record IDs are 1..10, not 0..9, so we subtract one.
func grouperTimeX(x int) time.Time {
t, err := time.ParseInLocation(time.RFC3339, sql1TestsGrouper.Table.rows[0][x-1][5].(string), time.UTC)
t, err := parser.ConvertStringToTimestamp(sql1TestsGrouper.Table.rows[0][x-1][5].(string))
if err != nil {
panic(fmt.Sprintf("failed to parse time for id %d", x))
}