remove non-presentational calls to (time.Time).UTC

Introduce `driver.Date` so we can print dates properly.

gossip/simulation/gossip.go

@@ -258,7 +258,7 @@ func outputDotFile(dotFN string, cycle int, network *simulation.Network, edgeSet
 func main() {
 	// Seed the random number generator for non-determinism across
 	// multiple runs.
-	rand.Seed(time.Now().UTC().UnixNano())
+	rand.Seed(time.Now().UnixNano())
 
 	if f := flag.Lookup("alsologtostderr"); f != nil {
 		fmt.Println("Starting simulation. Add -alsologtostderr to see progress.")

sql/driver/conn.go

@@ -20,6 +20,7 @@ package driver
 import (
 	"database/sql/driver"
 	"errors"
+	"fmt"
 	"strings"
 
 	"github.com/cockroachdb/cockroach/util"
@@ -162,11 +163,11 @@ func (c *conn) applySettings(params map[string]string) error {
 		name     string
 		operator string
 	}{
-		{name: "DATABASE", operator: " = "},
-		{name: "TIME ZONE", operator: " "},
+		{name: "DATABASE", operator: "="},
+		{name: "TIME ZONE", operator: ""},
 	} {
 		if val, ok := params[strings.ToLower(strings.Replace(setting.name, " ", "_", -1))]; ok {
-			commands = append(commands, strings.Join([]string{"SET " + setting.name, val}, setting.operator))
+			commands = append(commands, fmt.Sprintf("SET %s %s \"%s\"", setting.name, setting.operator, val))
 		}
 	}
 	_, err := c.Exec(strings.Join(commands, ";"), nil)

sql/driver/driver_test.go

@@ -27,13 +27,22 @@ import (
 
 	"github.com/cockroachdb/cockroach/security"
 	"github.com/cockroachdb/cockroach/server"
+	"github.com/cockroachdb/cockroach/sql/driver"
 	"github.com/cockroachdb/cockroach/testutils"
 	"github.com/cockroachdb/cockroach/util/leaktest"
 )
 
-func setup(t *testing.T) (*server.TestServer, *sql.DB) {
+func setup(t *testing.T, loc *time.Location) (*server.TestServer, *sql.DB) {
 	s := server.StartTestServer(nil)
-	db, err := sql.Open("cockroach", "https://root@"+s.ServingAddr()+"?certs=test_certs")
+	db, err := sql.Open("cockroach",
+		fmt.Sprintf(
+			"https://%s@%s?certs=%s&time_zone=%s",
+			security.RootUser,
+			s.ServingAddr(),
+			security.EmbeddedCertsDir,
+			loc,
+		),
+	)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -45,14 +54,71 @@ func cleanup(s *server.TestServer, db *sql.DB) {
 	s.Stop()
 }
 
+func TestDates(t *testing.T) {
+	defer leaktest.AfterTest(t)
+
+	// From https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
+	locationNames := []string{
+		"Canada/Newfoundland", // half-hour zone
+		"Europe/London",       // same as UTC, except for DST
+		"Pacific/Kiritimati",  // maximum positive offset
+		"Pacific/Midway",      // maximum negative offset
+		"US/Pacific",          // negative offset
+		"UTC",
+	}
+
+	for _, locationName := range locationNames {
+		loc, err := time.LoadLocation(locationName)
+		if err != nil {
+			t.Error(err)
+			continue
+		}
+
+		s, db := setup(t, loc)
+		defer cleanup(s, db)
+
+		var date *driver.Date
+		for _, year := range []int{
+			1200, // distant past
+			2020, // present day, for DST rules
+			4000, // distant future
+		} {
+			for _, month := range []time.Month{
+				time.December, // winter
+				time.August,   // summer
+			} {
+				for hour := 0; hour < 24; hour++ {
+					timestamp := time.Date(year, month, 20, hour, 34, 45, 123, loc)
+
+					if err := db.QueryRow("SELECT $1::DATE", timestamp).Scan(&date); err != nil {
+						t.Fatal(err)
+					}
+
+					if expected := driver.MakeDate(timestamp); !date.Equal(expected.Time) {
+						t.Fatalf("expected date to be truncated to:\n%s\nbut got:\n%s", expected, date)
+					}
+				}
+			}
+		}
+	}
+}
+
 func TestPlaceholders(t *testing.T) {
 	defer leaktest.AfterTest(t)
-	s, db := setup(t)
+
+	// loc is selected so that timeVal below maps to a different date in
+	// loc and UTC.
+	loc, err := time.LoadLocation("Pacific/Midway")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	s, db := setup(t, loc)
 	defer cleanup(s, db)
 
 	year, month, day := 3015, time.August, 30
-	timeVal := time.Date(year, month, day, 3, 34, 45, 345670000, time.UTC)
-	dateVal := time.Date(year, month, day, 0, 0, 0, 0, time.UTC)
+	timeVal := time.Date(year, month, day, 3, 34, 45, 345670000, loc)
+	dateVal := driver.Date{Time: time.Date(year, month, day, 0, 0, 0, 0, time.UTC)}
 	intervalVal, err := time.ParseDuration("34h2s")
 	if err != nil {
 		t.Fatal(err)
@@ -93,7 +159,7 @@ CREATE TABLE t.alltypes (
 		d sql.NullString
 		e sql.NullBool
 		f *time.Time
-		g *time.Time
+		g *driver.Date
 		h *time.Duration
 	)
 
@@ -169,8 +235,12 @@ CREATE TABLE t.alltypes (
 		}
 
 		if !(a == 123 && b.Float64 == 3.4 && c.String == "blah" && d.String == "foo" &&
-			e.Bool && f.Equal(timeVal) && g.Equal(dateVal) && *h == intervalVal) {
-			t.Errorf("got unexpected results: %+v", []interface{}{a, b, c, d, e, f, g, h})
+			e.Bool && f.Equal(timeVal) && g.Equal(dateVal.Time) && *h == intervalVal) {
+			t.Errorf(
+				"expected:\n%+v\ngot:\n%+v",
+				[]interface{}{123, 3.4, "blah", "foo", true, timeVal, dateVal, intervalVal},
+				[]interface{}{a, b, c, d, e, f, g, h},
+			)
 		}
 
 		rows.Next()
@@ -183,7 +253,11 @@ CREATE TABLE t.alltypes (
 		}
 
 		if !(a == 456 && !b.Valid && !c.Valid && !d.Valid && !e.Valid && f == nil && g == nil && h == nil) {
-			t.Errorf("got unexpected results: %+v", []interface{}{a, b, c, d, e, f, g, h})
+			t.Errorf(
+				"expected:\n%+v\ngot:\n%+v",
+				[]interface{}{123, "<NOT NULL>", "<NOT NULL>", "<NOT NULL>", "<NOT NULL>", "<NULL>", "<NULL>", "<NULL>"},
+				[]interface{}{a, b, c, d, e, f, g, h},
+			)
 		}
 
 		if rows.Next() {
@@ -213,7 +287,11 @@ CREATE TABLE t.alltypes (
 		}
 
 		if !(a == 456 && !b.Valid && !c.Valid && !d.Valid && !e.Valid && f == nil && g == nil && h == nil) {
-			t.Errorf("got unexpected results: %+v", []interface{}{a, b, c, d, e, f, g, h})
+			t.Errorf(
+				"expected:\n%+v\ngot:\n%+v",
+				[]interface{}{123, "<NOT NULL>", "<NOT NULL>", "<NOT NULL>", "<NOT NULL>", "<NULL>", "<NULL>", "<NULL>"},
+				[]interface{}{a, b, c, d, e, f, g, h},
+			)
 		}
 
 		if rows.Next() {
@@ -415,7 +493,7 @@ func concurrentIncrements(db *sql.DB, t *testing.T) {
 // are serializable and performant even at the SQL layer.
 func TestConcurrentIncrements(t *testing.T) {
 	defer leaktest.AfterTest(t)
-	s, db := setup(t)
+	s, db := setup(t, time.Local)
 	defer cleanup(s, db)
 
 	if _, err := db.Exec(`CREATE DATABASE t`); err != nil {

sql/driver/wire.go

@@ -50,18 +50,39 @@ func makeDatum(val driver.Value) (Datum, error) {
 	case string:
 		datum.Payload = &Datum_StringVal{t}
 	case time.Time:
-		// Send absolute time devoid of time-zone.
 		timestamp := Timestamp(t)
 		datum.Payload = &Datum_TimeVal{
 			&timestamp,
 		}
+	case Date:
+		timestamp := Timestamp(t.Time)
+		datum.Payload = &Datum_DateVal{
+			&timestamp,
+		}
 	default:
 		return datum, util.Errorf("unsupported type %T", t)
 	}
 
 	return datum, nil
 }
 
+// Date wraps time.Time and provides a custom String() method.
+type Date struct {
+	time.Time // Must always be UTC!
+}
+
+// MakeDate constructs a Date from a time.Time.
+func MakeDate(t time.Time) Date {
+	year, month, day := t.Date()
+	return Date{Time: time.Date(year, month, day, 0, 0, 0, 0, time.UTC)}
+}
+
+// String returns the underlying time formatted using the format string
+// "2006-01-02".
+func (d Date) String() string {
+	return d.Format("2006-01-02")
+}
+
 // Value implements the driver.Valuer interface.
 func (d Datum) Value() (driver.Value, error) {
 	var val driver.Value
@@ -80,9 +101,9 @@ func (d Datum) Value() (driver.Value, error) {
 	case *Datum_StringVal:
 		val = t.StringVal
 	case *Datum_DateVal:
-		val = t.DateVal.GoTime().UTC()
+		val = MakeDate(t.DateVal.GoTime())
 	case *Datum_TimeVal:
-		val = t.TimeVal.GoTime().UTC()
+		val = t.TimeVal.GoTime()
 	case *Datum_IntervalVal:
 		val = time.Duration(t.IntervalVal)
 	default:
@@ -92,9 +113,11 @@ func (d Datum) Value() (driver.Value, error) {
 	return val, nil
 }
 
-// GoTime converts the timestamp to a time.Time.
+// GoTime returns the receiver as a time.Time in UTC. It is critical
+// that the time.Time returned is in UTC, because that is the
+// storage/wire contract for dates and times.
 func (t Datum_Timestamp) GoTime() time.Time {
-	return time.Unix(t.Sec, int64(t.Nsec))
+	return time.Unix(t.Sec, int64(t.Nsec)).UTC()
 }
 
 // Timestamp converts a time.Time to a timestamp.

sql/executor.go

@@ -337,9 +337,9 @@ func (p parameters) Arg(name string) (parser.Datum, bool) {
 	case *driver.Datum_StringVal:
 		return parser.DString(t.StringVal), true
 	case *driver.Datum_DateVal:
-		return parser.DTimestamp{Time: t.DateVal.GoTime().UTC()}, true
+		return parser.MakeDDate(t.DateVal.GoTime()), true
 	case *driver.Datum_TimeVal:
-		return parser.DTimestamp{Time: t.TimeVal.GoTime().UTC()}, true
+		return parser.DTimestamp{Time: t.TimeVal.GoTime()}, true
 	case *driver.Datum_IntervalVal:
 		return parser.DInterval{Duration: time.Duration(t.IntervalVal)}, true
 	default:

sql/parser/builtins.go

@@ -436,7 +436,7 @@ var builtins = map[string][]builtin{
 			types:      typeList{},
 			returnType: DummyDate,
 			fn: func(e EvalContext, args DTuple) (Datum, error) {
-				return DDate{Time: e.StmtTimestamp.Truncate(24 * time.Hour)}, nil
+				return MakeDDate(e.StmtTimestamp.Time), nil
 			},
 		},
 	},

sql/parser/datum.go

@@ -335,7 +335,13 @@ func (d DBytes) String() string {
 
 // DDate is the date Datum.
 type DDate struct {
-	time.Time
+	time.Time // Must always be UTC!
+}
+
+// MakeDDate constructs a DDate from a time.Time.
+func MakeDDate(t time.Time) DDate {
+	year, month, day := t.Date()
+	return DDate{Time: time.Date(year, month, day, 0, 0, 0, 0, time.UTC)}
 }
 
 // Type implements the Datum interface.

sql/parser/eval.go

@@ -560,7 +560,11 @@ type EvalContext struct {
 	GetLocation   func() (*time.Location, error)
 }
 
-var defaultContext EvalContext
+var defaultContext = EvalContext{
+	GetLocation: func() (*time.Location, error) {
+		return time.UTC, nil
+	},
+}
 
 // EvalExpr evaluates an SQL expression. Expression evaluation is a mostly
 // straightforward walk over the parse tree. The only significant complexity is
@@ -1129,7 +1133,11 @@ func (ctx EvalContext) evalCastExpr(expr *CastExpr) (Datum, error) {
 		case DString:
 			return ParseDate(d)
 		case DTimestamp:
-			return DDate{Time: d.Truncate(24 * time.Hour)}, nil
+			loc, err := ctx.GetLocation()
+			if err != nil {
+				return DNull, err
+			}
+			return MakeDDate(d.Time.In(loc)), nil
 		}
 
 	case *TimestampType:
@@ -1239,7 +1247,7 @@ func ParseDate(s DString) (DDate, error) {
 		return DummyDate, err
 	}
 
-	return DDate{Time: t}, nil
+	return MakeDDate(t), nil
 }
 
 // ParseTimestamp parses the timestamp.
@@ -1263,7 +1271,7 @@ func (ctx EvalContext) ParseTimestamp(s DString) (DTimestamp, error) {
 	} {
 		var t time.Time
 		if t, err = time.ParseInLocation(format, str, loc); err == nil {
-			return DTimestamp{Time: t.UTC()}, nil
+			return DTimestamp{Time: t}, nil
 		}
 	}
 

sql/parser/eval_test.go

@@ -251,18 +251,18 @@ func TestEvalExpr(t *testing.T) {
 		{`'2010-09-28'::timestamp`, `2010-09-28 00:00:00+00:00`},
 		{`('2010-09-28 12:00:00.1'::timestamp)::date`, `2010-09-28`},
 		{`'2010-09-28 12:00:00.1'::timestamp`, `2010-09-28 12:00:00.1+00:00`},
-		{`'2010-09-28 12:00:00.1+02:00'::timestamp`, `2010-09-28 10:00:00.1+00:00`},
-		{`'2010-09-28 12:00:00.1-07:00'::timestamp`, `2010-09-28 19:00:00.1+00:00`},
+		{`'2010-09-28 12:00:00.1+02:00'::timestamp`, `2010-09-28 12:00:00.1+02:00`},
+		{`'2010-09-28 12:00:00.1-07:00'::timestamp`, `2010-09-28 12:00:00.1-07:00`},
 		{`('2010-09-28'::date)::timestamp`, `2010-09-28 00:00:00+00:00`},
 		{`'12h2m1s23ms'::interval`, `12h2m1.023s`},
 		{`1::interval`, `1ns`},
 		{`'2010-09-28'::date + '12h2m'::interval`, `2010-09-28 12:02:00+00:00`},
 		{`'12h2m'::interval + '2010-09-28'::date`, `2010-09-28 12:02:00+00:00`},
 		{`'2010-09-28'::date - '12h2m'::interval`, `2010-09-27 11:58:00+00:00`},
 		{`'2010-09-28'::date - '2010-10-21'::date`, `-552h0m0s`},
-		{`'2010-09-28 12:00:00.1-04:00'::timestamp + '12h2m'::interval`, `2010-09-29 04:02:00.1+00:00`},
-		{`'12h2m'::interval + '2010-09-28 12:00:00.1-04:00'::timestamp`, `2010-09-29 04:02:00.1+00:00`},
-		{`'2010-09-28 12:00:00.1-04:00'::timestamp - '12h2m'::interval`, `2010-09-28 03:58:00.1+00:00`},
+		{`'2010-09-28 12:00:00.1-04:00'::timestamp + '12h2m'::interval`, `2010-09-29 00:02:00.1-04:00`},
+		{`'12h2m'::interval + '2010-09-28 12:00:00.1-04:00'::timestamp`, `2010-09-29 00:02:00.1-04:00`},
+		{`'2010-09-28 12:00:00.1-04:00'::timestamp - '12h2m'::interval`, `2010-09-27 23:58:00.1-04:00`},
 		{`'2010-09-28 12:00:00.1-04:00'::timestamp - '2010-09-28 12:00:00.1+00:00'::timestamp`, `4h0m0s`},
 		{`'2010-09-28 12:00:00.1-04:00'::timestamp - '2010-09-28'::date`, `16h0m0.1s`},
 		{`'2010-09-28'::date - '2010-09-28 12:00:00.1-04:00'::timestamp`, `-16h0m0.1s`},

sql/scan.go

@@ -699,6 +699,12 @@ func (n *scanNode) explainDebug(endOfRow, outputRow bool) {
 	if n.implicitVals != nil {
 		n.row[2] = parser.DString(prettyKeyVals(n.implicitVals))
 	} else {
+		// This conversion to DString is odd. `n.explainValue` is already a
+		// `Datum`, but logic_test currently expects EXPLAIN DEBUG output
+		// to come out formatted using `encodeSQLString`. This is not
+		// consistent across all printing of strings in logic_test, though.
+		// TODO(tamird/pmattis): figure out a consistent story for string
+		// printing in logic_test.
 		n.row[2] = parser.DString(n.explainValue.String())
 	}
 	if endOfRow {

sql/table.go

@@ -369,7 +369,7 @@ func decodeTableKey(valType parser.Datum, key []byte) (parser.Datum, []byte, err
 		return parser.DBytes(r), rkey, err
 	case parser.DDate:
 		rkey, t, err := encoding.DecodeTime(key)
-		return parser.DDate{Time: t}, rkey, err
+		return parser.MakeDDate(t), rkey, err
 	case parser.DTimestamp:
 		rkey, t, err := encoding.DecodeTime(key)
 		return parser.DTimestamp{Time: t}, rkey, err
@@ -516,7 +516,7 @@ func unmarshalColumnValue(kind ColumnType_Kind, value *roachpb.Value) (parser.Da
 		if err != nil {
 			return nil, err
 		}
-		return parser.DDate{Time: v}, nil
+		return parser.MakeDDate(v), nil
 	case ColumnType_TIMESTAMP:
 		v, err := value.GetTime()
 		if err != nil {