Support parsing pgwire timestamps as they are sent by lib/pq

sql/pgwire/types.go

@@ -236,6 +236,25 @@ func formatTs(t time.Time) (b []byte) {
 	return b
 }
 
+// parseTs parses timestamps in any of the formats that Postgres accepts over
+// the wire protocol.
+//
+// Postgres is lenient in what it accepts as a timestamp, so we must also be
+// lenient. As new drivers are used with CockroachDB and formats are found that
+// we don't support but Postgres does, add them here. Then create an integration
+// test for the driver and add a case to TestParseTs.
+func parseTs(str string) (time.Time, error) {
+	// RFC3339Nano is sent by github.com/lib/pq (go).
+	if ts, err := time.Parse(time.RFC3339Nano, str); err == nil {
+		return ts, nil
+	}
+
+	// pq.ParseTimestamp parses the timestamp format that both Postgres and
+	// CockroachDB send in responses, so this allows roundtripping of the encoded
+	// timestamps that we send.
+	return pq.ParseTimestamp(nil, str)
+}
+
 var (
 	oidToDatum = map[oid.Oid]parser.Datum{
 		oid.T_bool:      parser.DummyBool,
@@ -416,7 +435,7 @@ func decodeOidDatum(id oid.Oid, code formatCode, b []byte) (parser.Datum, error)
 	case oid.T_timestamp:
 		switch code {
 		case formatText:
-			ts, err := parseTimestamp(string(b))
+			ts, err := parseTs(string(b))
 			if err != nil {
 				return d, fmt.Errorf("could not parse string %q as timestamp", b)
 			}
@@ -427,7 +446,7 @@ func decodeOidDatum(id oid.Oid, code formatCode, b []byte) (parser.Datum, error)
 	case oid.T_date:
 		switch code {
 		case formatText:
-			ts, err := parseTimestamp(string(b))
+			ts, err := parseTs(string(b))
 			if err != nil {
 				return d, fmt.Errorf("could not parse string %q as date", b)
 			}
@@ -441,14 +460,3 @@ func decodeOidDatum(id oid.Oid, code formatCode, b []byte) (parser.Datum, error)
 	}
 	return d, nil
 }
-
-func parseTimestamp(str string) (time.Time, error) {
-	// TODO(dan): The cockroach/pq driver encodes timestamps using
-	// time.RFC3339Nano, yet it cannot parse those timestamps using
-	// pq.ParseTimestamp. We should either fix pq.ParseTimestamp or replace this
-	// comment with one explaining the discrepancy.
-	if ts, err := time.Parse(time.RFC3339Nano, str); err == nil {
-		return ts, nil
-	}
-	return pq.ParseTimestamp(nil, str)
-}

sql/pgwire/types_test.go

@@ -0,0 +1,66 @@
+// Copyright 2016 The Cockroach Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+//
+// Author: Dan Harrison ([email protected])
+
+package pgwire
+
+import (
+	"testing"
+	"time"
+
+	"github.com/cockroachdb/cockroach/util/leaktest"
+)
+
+// The assertions in this test should also be caught by the integration tests on
+// various drivers.
+func TestParseTs(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	var parseTsTests = []struct {
+		strTimestamp string
+		expected     time.Time
+	}{
+		// time.RFC3339Nano for github.com/lib/pq.
+		{"2006-07-08T00:00:00.000000123Z", time.Date(2006, 7, 8, 0, 0, 0, 123, time.FixedZone("UTC", 0))},
+
+		// The format accepted by pq.ParseTimestamp.
+		{"2001-02-03 04:05:06.123-07", time.Date(2001, time.February, 3, 4, 5, 6, 123000000, time.FixedZone("", -7*60*60))},
+	}
+
+	for i, test := range parseTsTests {
+		parsed, err := parseTs(test.strTimestamp)
+		if err != nil {
+			t.Errorf("%d could not parse [%s]: %v", i, test.strTimestamp, err)
+			continue
+		}
+		if !parsed.Equal(test.expected) {
+			t.Errorf("%d parsing [%s] got [%s] expected [%s]", i, test.strTimestamp, parsed, test.expected)
+		}
+	}
+}
+
+func TestTimestampRoundtrip(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	ts := time.Date(2006, 7, 8, 0, 0, 0, 123, time.FixedZone("UTC", 0))
+
+	encoded := string(formatTs(ts))
+	decoded, err := parseTs(encoded)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !ts.Equal(decoded) {
+		t.Fatalf("timestamp did not roundtrip got [%s] expected [%s]", decoded, ts)
+	}
+}