Not null Timestamp/Timestamptz fields are not casted to time.Time by default

[okanmete]

Version

1.23.0

What happened?

Hi, Recently, I upgraded my "pgx" version from v4 to v5. I was using sqlc v1.17.2 and also upgraded it to v1.23.0 . My problem is in pgx4, the "not null timestamptz/timestamp" types are generated as time.Time and nullable ones are generated as sqlNullTime as expected. However, after I switch to pgx5, regardles of a timestamptz/timestamp field is null or not-null, the golang object is always generated as pgType.Timestamp, pgType.Timestamptz.

Indeed, when I apply the below overrides in my sqlc.yaml file, the problem is resolved and nullable types are generated as pgType.Timestamptz, not null types are generated as time.Time as expected. My question is, should not it be something happening by default ?

    overrides:
      - db_type: "pg_catalog.timestamp"
        go_type:
          type: "Time"
          import: "time"
      - db_type: "timestamp"
        go_type:
          type: "Time"
          import: "time"
      - db_type: "pg_catalog.timestamptz"
        go_type:
          type: "Time"
          import: "time"
      - db_type: "timestamptz"
        go_type:
          type: "Time"
          import: "time"

Relevant log output

No response

Database schema

No response

SQL queries

No response

Configuration

No response

Playground URL

No response

What operating system are you using?

macOS

What database engines are you using?

PostgreSQL

What type of code are you generating?

Go

[andrewmbenton]

I can't say for sure because I haven't tested this in a real-world situation and I wasn't around when the pgx/v5 defaults were decided, but I suspect there may be a compatibility issue between the way pgx/v5 handles the PostgreSQL date/time types and time.Time. This old pull request alludes to an issue like that for time and timetz` PostgreSQL types: #1623.

With overrides you can certainly get sqlc to use the types you want, but have you tried using generated code to retrieve rows with timestamps using pgx/v5 and time.Time and can confirm that scanning into your structs works correctly?

In any case I don't think we'd change the defaults at this point because they've been in the wild for too long.

[okanmete]

Yeah, I did a comprehensive testing here. In both default and overrided version, I read the data as expected. This is kind of preferance. I prefer having not-null fields to be generated as time.Time so I will keep using overrides here. Thanks!!