Merge #31758

31758: sql: Generalize date/time parsing r=bobvawter a=bobvawter

sql: Generalize date/time parsing

The current date/time parsing code relies on `time.ParseInLocation()`. It does
not support all of the various date/time formats accepted by PostgreSQL and
also requires multiple invocation to try the various date/time formats that we
do accept.

This change updates the date/time parsing code with a new implementation that
does not delegate to `time.ParseInLocation()` and is able to parse all
supported formats in a single pass.

In order to support parsing named timezones like `America/New_York`, we
delegate to `time.LoadLocation()` as we did previously.  `LoadLocation()` is
rather expensive, since it looks for tzinfo files on disk every time it is
invoked. A per-node, in-memory cache has been added to amortize this overhead.
Per #31978, the tzinfo used on each node could already be inconsistent,
depending on the tzinfo files present in the underlying OS.

The following table compares the new `ParseTimestamp()` function to calling
`ParseInLocation()`.  While it is true that `ParseInLocation()` is generally
faster for any given pattern, the current parsing code must call it repeatedly,
trying each supported date format until one succeeds. The test with the named
timezone also shows the significant overhead of calling `LoadLocation()`.

```
2003-06-12/ParseTimestamp-8             10000000               122 ns/op          81.53 MB/s
2003-06-12/ParseInLocation-8            30000000                35.6 ns/op       281.29 MB/s
2003-06-12_01:02:03/ParseTimestamp-8            10000000               163 ns/op         116.45 MB/s
2003-06-12_01:02:03/ParseInLocation-8           30000000                54.4 ns/op       349.16 MB/s
2003-06-12_04:05:06.789-04:00/ParseTimestamp-8          10000000               238 ns/op         121.69 MB/s
2003-06-12_04:05:06.789-04:00/ParseInLocation-8         10000000               161 ns/op         180.05 MB/s
2000-01-01T02:02:02.567+09:30/ParseTimestamp-8           5000000               233 ns/op         124.01 MB/s
2000-01-01T02:02:02.567+09:30/ParseInLocation-8         10000000               158 ns/op         182.41 MB/s
2003-06-12_04:05:06.789_America/New_York/ParseTimestamp-8                3000000               475 ns/op          84.06 MB/s
2003-06-12_04:05:06.789_America/New_York/ParseInLocation-8                200000              7313 ns/op           3.15 MB/s
```

The tests in `parsing_test.go` have an optional mode to cross-check the test
data aginst a PostgreSQL server.  This is useful for developing, but is not
part of the automated build.

Parsing of BC dates is supported, #28099 could then be completed by changing
the date-formatting code to print a BC date.

This change would allow #30697 (incomplete handling of datestyle) to be
re-evaluated, since the parser does allow configuration of YMD, DMY, or MDY
input styles.

Resolves #27500
Resolves #27501
Resolves #31954

Release note (sql change): A wider variety of date, time, and timestamp formats
are now accepted by the SQL frontend.

Release note (bug fix): Prepared statements that bind temporal values now
respect the session's timezone setting. Previously, bound temporal values were
always interpreted as though the session time zone were UTC.

Release note (backward-incompatible change): Timezone abbreviations, such as
EST, are no longer allowed when parsing or converting to a date/time type.
Previously, an abbreviation would be accepted if it were an alias for the
session's timezone.


Co-authored-by: Bob Vawter <bob@cockroachlabs.com>

pkg/ccl/importccl/read_import_mysql_test.go

@@ -275,7 +275,7 @@ func TestMysqlValueToDatum(t *testing.T) {
  defer leaktest.AfterTest(t)()
 
  date := func(s string) tree.Datum {
- d, err := tree.ParseDDate(s, nil)
+ d, err := tree.ParseDDate(nil, s)
  if err != nil {
  t.Fatal(err)
  }

pkg/sql/conn_executor_prepare.go

@@ -326,14 +326,17 @@ func (ex *connExecutor) execBind(
  "expected %d arguments, got %d", numQArgs, len(bindCmd.Args)))
  }
 
+ ptCtx := tree.NewParseTimeContext(ex.sessionData.DurationAdditionMode,
+ ex.state.sqlTimestamp.In(ex.sessionData.DataConversion.Location))
+
  for i, arg := range bindCmd.Args {
  k := strconv.Itoa(i + 1)
  t := ps.InTypes[i]
  if arg == nil {
  // nil indicates a NULL argument value.
  qargs[k] = tree.DNull
  } else {
- d, err := pgwirebase.DecodeOidDatum(t, qArgFormatCodes[i], arg)
+ d, err := pgwirebase.DecodeOidDatum(ptCtx, t, qArgFormatCodes[i], arg)
  if err != nil {
  if _, ok := err.(*pgerror.Error); ok {
  return retErr(err)

pkg/sql/logictest/testdata/logic_test/datetime

@@ -646,14 +646,13 @@ SELECT '2015-08-25 05:45:45.53453'::timestamp
 statement ok
 SET TIME ZONE 'Europe/Rome'
 
-query T
+query error pq: unimplemented: timestamp abbreviations not supported
 SELECT '2015-08-25 05:45:45.53453 CET'::timestamptz WHERE false
-----
 
 statement ok
 SET TIME ZONE +1
 
-query error unknown zone "CET"
+query error pq: unimplemented: timestamp abbreviations not supported
 SELECT '2015-08-25 05:45:45.53453 CET'::timestamptz WHERE false
 
 query T
@@ -690,7 +689,7 @@ SELECT '2015-08-25 05:45:45-01:00'::timestamptz
 statement ok
 SET TIMEZONE = +2
 
-query error unknown zone "CET"
+query error pq: unimplemented: timestamp abbreviations not supported
 SELECT '2015-08-25 05:45:45.53453 CET'::timestamptz WHERE false
 
 query T

pkg/sql/logictest/testdata/logic_test/typing

@@ -136,11 +136,11 @@ SELECT ts FROM untyped WHERE ts != '2015-09-18 00:00:00'
 
 # Regression tests for #15050
 
-statement error error type checking constant value: could not parse "2017-04-18 18:00" as type timestamp
-CREATE TABLE t15050a (c DECIMAL DEFAULT CASE WHEN now() < '2017-04-18 18:00' THEN 2 ELSE 2 END);
+statement error pq: error type checking constant value: could not parse "Not Timestamp" as type timestamp
+CREATE TABLE t15050a (c DECIMAL DEFAULT CASE WHEN now() < 'Not Timestamp' THEN 2 ELSE 2 END);
 
-statement error error type checking constant value: could not parse "2017-04-18 18:00" as type timestamp
-CREATE TABLE t15050b (c DECIMAL DEFAULT IF(now() < '2017-04-18 18:00', 2, 2));
+statement error pq: error type checking constant value: could not parse "Not Timestamp" as type timestamp
+CREATE TABLE t15050b (c DECIMAL DEFAULT IF(now() < 'Not Timestamp', 2, 2));
 
 # Regression tests for #15632
 

pkg/sql/pgwire/binary_test.go

@@ -84,7 +84,7 @@ func testBinaryDatumType(t *testing.T, typ string, datumConstructor func(val str
  }
 
  datum, err := pgwirebase.DecodeOidDatum(
- oid, pgwirebase.FormatBinary, got[4:],
+ evalCtx, oid, pgwirebase.FormatBinary, got[4:],
  )
  if err != nil {
  t.Fatalf("unable to decode %v: %s", got[4:], err)
@@ -123,7 +123,7 @@ func TestBinaryTimestamp(t *testing.T) {
 func TestBinaryTimestampTZ(t *testing.T) {
  defer leaktest.AfterTest(t)()
  testBinaryDatumType(t, "timestamptz", func(val string) tree.Datum {
- tstz, err := tree.ParseDTimestampTZ(val, time.UTC, time.Microsecond)
+ tstz, err := tree.ParseDTimestampTZ(nil, val, time.Microsecond)
  if err != nil {
  t.Fatal(err)
  }
@@ -145,7 +145,7 @@ func TestBinaryInterval(t *testing.T) {
 func TestBinaryDate(t *testing.T) {
  defer leaktest.AfterTest(t)()
  testBinaryDatumType(t, "date", func(val string) tree.Datum {
- d, err := tree.ParseDDate(val, time.UTC)
+ d, err := tree.ParseDDate(nil, val)
  if err != nil {
  t.Fatal(err)
  }
@@ -156,7 +156,7 @@ func TestBinaryDate(t *testing.T) {
 func TestBinaryTime(t *testing.T) {
  defer leaktest.AfterTest(t)()
  testBinaryDatumType(t, "time", func(val string) tree.Datum {
- d, err := tree.ParseDTime(val)
+ d, err := tree.ParseDTime(nil, val)
  if err != nil {
  t.Fatal(err)
  }
@@ -232,7 +232,7 @@ func TestBinaryIntArray(t *testing.T) {
 
  b := buf.wrapped.Bytes()
 
- got, err := pgwirebase.DecodeOidDatum(oid.T__int8, pgwirebase.FormatBinary, b[4:])
+ got, err := pgwirebase.DecodeOidDatum(nil, oid.T__int8, pgwirebase.FormatBinary, b[4:])
  if err != nil {
  t.Fatal(err)
  }
@@ -304,7 +304,7 @@ func TestRandomBinaryDecimal(t *testing.T) {
  if got := buf.wrapped.Bytes(); !bytes.Equal(got, test.Expect) {
  t.Errorf("%q:\n\t%v found,\n\t%v expected", test.In, got, test.Expect)
  } else if datum, err := pgwirebase.DecodeOidDatum(
- oid.T_numeric, pgwirebase.FormatBinary, got[4:],
+ nil, oid.T_numeric, pgwirebase.FormatBinary, got[4:],
  ); err != nil {
  t.Errorf("%q: unable to decode %v: %s", test.In, got[4:], err)
  } else if dec.Compare(evalCtx, datum) != 0 {

pkg/sql/pgwire/pgerror/errors.go

@@ -195,6 +195,15 @@ func UnimplementedWithIssueError(issue int, msg string) error {
  return err.SetHintf("See: https://github.com/cockroachdb/cockroach/issues/%d", issue)
 }
 
+// UnimplementedWithIssueDetailError constructs an error with the given message
+// and a link to the passed issue. Recorded as "#<issue>.detail" in tracking.
+// This is useful when we need an extra axis of information to drill down into.
+func UnimplementedWithIssueDetailError(issue int, detail, msg string) error {
+ err := NewErrorWithDepthf(1, CodeFeatureNotSupportedError, "unimplemented: %s", msg)
+ err.InternalCommand = fmt.Sprintf("#%d.%s", issue, detail)
+ return err.SetHintf("See: https://github.com/cockroachdb/cockroach/issues/%d", issue)
+}
+
 const unimplementedErrorHint = `This feature is not yet implemented in CockroachDB.
 
 Please check https://github.com/cockroachdb/cockroach/issues to check

pkg/sql/pgwire/pgwire_test.go

@@ -1144,6 +1144,45 @@ func (p preparedExecTest) RowsAffectedErr(err string) preparedExecTest {
  return p
 }
 
+// Verify that bound dates are evaluated using session timezone.
+func TestPGPrepareDate(t *testing.T) {
+ defer leaktest.AfterTest(t)()
+ s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
+ defer s.Stopper().Stop(context.TODO())
+
+ if _, err := db.Exec("CREATE TABLE test (t TIMESTAMPTZ)"); err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := db.Exec("SET TIME ZONE +08"); err != nil {
+ t.Fatal(err)
+ }
+
+ stmt, err := db.Prepare("INSERT INTO test VALUES ($1)")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := stmt.Exec("2018-01-01 12:34:56"); err != nil {
+ t.Fatal(err)
+ }
+
+ // Reset to UTC for the query.
+ if _, err := db.Exec("SET TIME ZONE UTC"); err != nil {
+ t.Fatal(err)
+ }
+
+ var ts time.Time
+ if err := db.QueryRow("SELECT t FROM test").Scan(&ts); err != nil {
+ t.Fatal(err)
+ }
+
+ exp := time.Date(2018, 1, 1, 4, 34, 56, 0, time.UTC)
+ if !exp.Equal(ts) {
+ t.Fatalf("expected %s, got %s", exp, ts)
+ }
+}
+
 func TestPGPreparedExec(t *testing.T) {
  defer leaktest.AfterTest(t)()
  var baseTest preparedExecTest

pkg/sql/pgwire/pgwirebase/encoding.go

@@ -190,8 +190,11 @@ func NewProtocolViolationErrorf(format string, args ...interface{}) error {
 }
 
 // DecodeOidDatum decodes bytes with specified Oid and format code into
-// a datum.
-func DecodeOidDatum(id oid.Oid, code FormatCode, b []byte) (tree.Datum, error) {
+// a datum. If the ParseTimeContext is nil, reasonable defaults
+// will be applied.
+func DecodeOidDatum(
+ ctx tree.ParseTimeContext, id oid.Oid, code FormatCode, b []byte,
+) (tree.Datum, error) {
  switch code {
  case FormatText:
  switch id {
@@ -238,30 +241,25 @@ func DecodeOidDatum(id oid.Oid, code FormatCode, b []byte) (tree.Datum, error) {
  }
  return tree.NewDBytes(tree.DBytes(res)), nil
  case oid.T_timestamp:
- d, err := tree.ParseDTimestamp(nil, string(b), time.Microsecond)
+ d, err := tree.ParseDTimestamp(ctx, string(b), time.Microsecond)
  if err != nil {
  return nil, errors.Errorf("could not parse string %q as timestamp", b)
  }
  return d, nil
  case oid.T_timestamptz:
- d, err := tree.ParseDTimestampTZ(string(b), time.UTC, time.Microsecond)
+ d, err := tree.ParseDTimestampTZ(ctx, string(b), time.Microsecond)
  if err != nil {
  return nil, errors.Errorf("could not parse string %q as timestamptz", b)
  }
  return d, nil
  case oid.T_date:
- ts, err := tree.ParseDTimestamp(nil, string(b), time.Microsecond)
+ d, err := tree.ParseDDate(ctx, string(b))
  if err != nil {
- res, err := tree.ParseDDate(string(b), time.UTC)
- if err != nil {
- return nil, errors.Errorf("could not parse string %q as date", b)
- }
- return res, nil
+ return nil, errors.Errorf("could not parse string %q as date", b)
  }
- daysSinceEpoch := ts.Unix() / secondsInDay
- return tree.NewDDate(tree.DDate(daysSinceEpoch)), nil
+ return d, nil
  case oid.T_time:
- d, err := tree.ParseDTime(string(b))
+ d, err := tree.ParseDTime(nil, string(b))
  if err != nil {
  return nil, errors.Errorf("could not parse string %q as time", b)
  }
@@ -513,7 +511,7 @@ func DecodeOidDatum(id oid.Oid, code FormatCode, b []byte) (tree.Datum, error) {
  return tree.ParseDJSON(string(b))
  default:
  if _, ok := types.ArrayOids[id]; ok {
- return decodeBinaryArray(b, code)
+ return decodeBinaryArray(ctx, b, code)
  }
  }
  default:
@@ -618,7 +616,7 @@ func pgBinaryToIPAddr(b []byte) (ipaddr.IPAddr, error) {
  }, nil
 }
 
-func decodeBinaryArray(b []byte, code FormatCode) (tree.Datum, error) {
+func decodeBinaryArray(ctx tree.ParseTimeContext, b []byte, code FormatCode) (tree.Datum, error) {
  hdr := struct {
  Ndims int32
  // Nullflag
@@ -648,7 +646,7 @@ func decodeBinaryArray(b []byte, code FormatCode) (tree.Datum, error) {
  return nil, err
  }
  buf := r.Next(int(vlen))
- elem, err := DecodeOidDatum(elemOid, code, buf)
+ elem, err := DecodeOidDatum(ctx, elemOid, code, buf)
  if err != nil {
  return nil, err
  }

pkg/sql/pgwire/types_test.go

@@ -136,7 +136,7 @@ func TestIntArrayRoundTrip(t *testing.T) {
 
  b := buf.wrapped.Bytes()
 
- got, err := pgwirebase.DecodeOidDatum(oid.T__int8, pgwirebase.FormatText, b[4:])
+ got, err := pgwirebase.DecodeOidDatum(nil, oid.T__int8, pgwirebase.FormatText, b[4:])
  if err != nil {
  t.Fatal(err)
  }
@@ -213,7 +213,7 @@ func TestByteArrayRoundTrip(t *testing.T) {
  b := buf.wrapped.Bytes()
  t.Logf("encoded: %v (%q)", b, b)
 
- got, err := pgwirebase.DecodeOidDatum(oid.T_bytea, pgwirebase.FormatText, b[4:])
+ got, err := pgwirebase.DecodeOidDatum(nil, oid.T_bytea, pgwirebase.FormatText, b[4:])
  if err != nil {
  t.Fatal(err)
  }
@@ -321,7 +321,7 @@ func benchmarkWriteString(b *testing.B, format pgwirebase.FormatCode) {
 }
 
 func benchmarkWriteDate(b *testing.B, format pgwirebase.FormatCode) {
- d, err := tree.ParseDDate("2010-09-28", time.UTC)
+ d, err := tree.ParseDDate(nil, "2010-09-28")
  if err != nil {
  b.Fatal(err)
  }
@@ -337,7 +337,7 @@ func benchmarkWriteTimestamp(b *testing.B, format pgwirebase.FormatCode) {
 }
 
 func benchmarkWriteTimestampTZ(b *testing.B, format pgwirebase.FormatCode) {
- tstz, err := tree.ParseDTimestampTZ("2010-09-28 12:00:00.1", time.UTC, time.Microsecond)
+ tstz, err := tree.ParseDTimestampTZ(nil, "2010-09-28 12:00:00.1", time.Microsecond)
  if err != nil {
  b.Fatal(err)
  }
@@ -470,7 +470,7 @@ func BenchmarkDecodeBinaryDecimal(b *testing.B) {
  b.ResetTimer()
  for i := 0; i < b.N; i++ {
  b.StartTimer()
- got, err := pgwirebase.DecodeOidDatum(oid.T_numeric, pgwirebase.FormatBinary, bytes)
+ got, err := pgwirebase.DecodeOidDatum(nil, oid.T_numeric, pgwirebase.FormatBinary, bytes)
  b.StopTimer()
  evalCtx := tree.NewTestingEvalContext(cluster.MakeTestingClusterSettings())
  defer evalCtx.Stop(context.Background())

pkg/sql/sem/tree/constant_test.go

@@ -199,7 +199,14 @@ func mustParseDBool(t *testing.T, s string) tree.Datum {
  return d
 }
 func mustParseDDate(t *testing.T, s string) tree.Datum {
- d, err := tree.ParseDDate(s, time.UTC)
+ d, err := tree.ParseDDate(nil, s)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return d
+}
+func mustParseDTime(t *testing.T, s string) tree.Datum {
+ d, err := tree.ParseDTime(nil, s)
  if err != nil {
  t.Fatal(err)
  }
@@ -213,7 +220,7 @@ func mustParseDTimestamp(t *testing.T, s string) tree.Datum {
  return d
 }
 func mustParseDTimestampTZ(t *testing.T, s string) tree.Datum {
- d, err := tree.ParseDTimestampTZ(s, time.UTC, time.Millisecond)
+ d, err := tree.ParseDTimestampTZ(nil, s, time.Millisecond)
  if err != nil {
  t.Fatal(err)
  }
@@ -239,6 +246,7 @@ var parseFuncs = map[types.T]func(*testing.T, string) tree.Datum{
  types.Bytes: func(t *testing.T, s string) tree.Datum { return tree.NewDBytes(tree.DBytes(s)) },
  types.Bool: mustParseDBool,
  types.Date: mustParseDDate,
+ types.Time: mustParseDTime,
  types.Timestamp: mustParseDTimestamp,
  types.TimestampTZ: mustParseDTimestampTZ,
  types.Interval: mustParseDInterval,
@@ -277,11 +285,11 @@ func TestStringConstantResolveAvailableTypes(t *testing.T) {
  },
  {
  c: tree.NewStrVal("2010-09-28 12:00:00.1"),
- parseOptions: typeSet(types.String, types.Bytes, types.Timestamp, types.TimestampTZ, types.Date),
+ parseOptions: typeSet(types.String, types.Bytes, types.Time, types.Timestamp, types.TimestampTZ, types.Date),
  },
  {
  c: tree.NewStrVal("2006-07-08T00:00:00.000000123Z"),
- parseOptions: typeSet(types.String, types.Bytes, types.Timestamp, types.TimestampTZ, types.Date),
+ parseOptions: typeSet(types.String, types.Bytes, types.Time, types.Timestamp, types.TimestampTZ, types.Date),
  },
  {
  c: tree.NewStrVal("PT12H2M"),