thanks for this comment, these one-liners are always worth so much when trying to understand what's going on

crazy definition language 😬

Might be a good opportunity to create a helper for this that auto does this while scanning

How would I do that? 🤔

I believe

type UTCTime struct{ *time.Time }

// Scan implements the Scanner interface.
func (nt *NullTime) Scan(value any) error {
	*nt.Time, _ = value.(time.Time).UTC()
	return nil
}

Took a bit of finesse, but I think this is how you do it:

package utctime

import (
	"database/sql"
	"database/sql/driver"
	"time"

	"github.com/sourcegraph/sourcegraph/lib/errors"
	"github.com/sourcegraph/sourcegraph/lib/pointers"
)

// Time is a wrapper around time.Time that implements the database/sql.Scanner
// and database/sql/driver.Valuer interfaces to serialize and deserialize time
// in UTC time zone.
type Time time.Time

// Now returns the current time in UTC.
func Now() Time { return Time(time.Now().UTC()) }

var _ sql.Scanner = (*Time)(nil)

func (t *Time) Scan(src any) error {
	if src == nil {
		return nil
	}
	if v, ok := src.(time.Time); ok {
		*t = Time(v.UTC())
		return nil
	}
	return errors.Newf("value %T is not time.Time", src)
}

var _ driver.Valuer = (*Time)(nil)

// Value must be called with a non-nil Time. driver.Valuer callers will first
// check that the value is non-nil, so this is safe.
func (t Time) Value() (driver.Value, error) {
	stdTime := t.AsTime()
	return *stdTime, nil
}

// AsTime returns the underlying time.Time value, or nil if it is nil.
func (t *Time) AsTime() *time.Time {
	if t == nil {
		return nil
	}
	// Ensure the time is in UTC.
	return pointers.Ptr((*time.Time)(t).UTC())
}

follow-up PR coming to use this for all time :) Thank you!

I'm curious why we're trying to build an ORM here 😬 So far in sg/sg we've mostly been quite explicit with the queries we write, and if we make our own ORM builder anyways, could we get away even cheaper if we just used the active record features of gorm directly?

In gorm, a lot of the options are automagical annotations (as you point out in https://github.com/sourcegraph/sourcegraph/pull/63703#discussion_r1669523483 quickly gets crazy), and the feature set is pretty broad. IMO this gives us some more control in a smaller API surface, which lines up with being explicit, without being super repetitive/error-prone to write, as I find some of our queries get. WDYT?

In particular this just takes what would be a huge list of if/else statements we would have to write several times for each of our CRUD resources, and allows it to be backed by a dedicated helper, so I don't think it's quite a full-fledged ORM

Not gonna line - literal SQL queries is far more readable than this when the entire query can be written at once, but I do see when there are lots if-conditions, this pattern/helper helps.

I think this is important to have because otherwise the AIP upsert pattern is far too annoying to build for many fields. FWIW, using -v in tests will dump the full query for every interaction in the test output

https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/cmd/enterprise-portal/internal/database/databasetest/databasetest.go?L85%3A1-87%3A3

also talked a little about it here: https://github.com/sourcegraph/sourcegraph/pull/63703#discussion_r1669536774

I did read through all (unresolved) comments :D any chance we can make this package to allow chained calls (similar to what GORM provides), e.g.

b := upsert.New(...).Fields(...)
if ... {
	b = b.Fields(...)
}

😂 maybe too fancy tho

I'm not sure that makes it nicer as now you have to worry about assignment etc 🤔 Additionally, the design right now uses a bit of generics which b.Fields will not allow

Ahh right... Go generics can't be used on methods (yet?), alright nvm 😬