jackc / pgx

PostgreSQL driver and toolkit for Go
MIT License
10.86k stars 846 forks source link

Constructor function for pgtype.Interval or additional comments #2111

Closed mateuszkowalke closed 3 months ago

mateuszkowalke commented 3 months ago

Is your feature request related to a problem? Please describe. Trying to use pgtype.Intervalconsumed way more time than I'm willing to admit ;) Missed the fact, that Valid field has to be explicitly set to true for it to work. Trying to use it in a query like WHERE <timestamp column> > now() - $1::INTERVAL gave no error.

Describe the solution you'd like Considering adding a NewInterval constructor function or adding comments for exported members related to pgtype.Interval.

Describe alternatives you've considered

Additional context

mateuszkowalke commented 3 months ago

More than happy to contribute after decision on which direction to go is made ;)

jackc commented 3 months ago

🤷 Needing to set Valid is a pattern first established by database/sql which has been continued by pgx. This affects almost every type in database/sql and pgx. If this was to be done, it should be done in a standard way for all types.

But I don't think it should be necessary. If you don't need the possibility of NULL, then you can directly use the underlying type instead of the database/sql or pgx wrapper. In this case that would be time.Duration.

mateuszkowalke commented 3 months ago

Thank you very much for explanation. I guess it's lack of knowledge on my part. Having given this a thought with this additional context, I suppose adding a comment similar to the ones found in database/sql for nullable wrappers might still be a good idea?But I don't think it should be necessary. If you don't need the possibility of NULL, then you can directly use the underlying type instead of the database/sql or pgx wrapper. In this case that would be time.Duration. Something like:

// NullString represents a string that may be null.
// NullString implements the [Scanner] interface so
// it can be used as a scan destination:
//
//  var s NullString
//  err := db.QueryRow("SELECT name FROM foo WHERE id=?", id).Scan(&s)
//  ...
//  if s.Valid {
//     // use s.String
//  } else {
//     // NULL value
//  }

I think it would help newbies like myself to quickly understand the reasoning behind these types.

jackc commented 3 months ago

Sure.