jackc / pgtype

MIT License
318 stars 112 forks source link

new: allow to override timezone in returned time.Time #215

Closed lansfy closed 3 months ago

lansfy commented 7 months ago

github.com/jackc/pgx and github.com/lib/pq behaviour for returned time.Time is different. pq uses timezone from connection and returns all time.Time in this timezone.

For example, next code:

package main

import (
    "context"
    "log"
    "time"

    "github.com/jackc/pgx/v4"
)

func main() {
    const connStr = "postgres://postgres:qwe123QWE@localhost:5432/postgres?sslmode=disable"
    conn, err := pgx.Connect(context.Background(), connStr)
    if err != nil {
        log.Fatal(err)
    }

    var tz string
    var tm time.Time
    err = conn.QueryRow(context.Background(), "select current_setting('TIMEZONE'), now()").Scan(&tz, &tm)
    if err != nil {
        log.Fatal(err)
    }
    log.Printf("current_setting('TIMEZONE'): %s, now(): %v\n", tz, tm)
}

print next string

2024/04/27 21:33:52 current_setting('TIMEZONE'): UTC, now(): 2024-04-27 21:33:52.853171 -0700 MST

but similar code with pq

package main

import (
    "context"
    "database/sql"
    "log"
    "time"

    _ "github.com/lib/pq"
)

func main() {
    const connStr = "postgres://postgres:qwe123QWE@localhost:5432/postgres?sslmode=disable"
    conn, err := sql.Open("postgres", connStr)
    if err != nil {
        log.Fatal(err)
    }

    var tz string
    var tm time.Time
    err = conn.QueryRow("select current_setting('TIMEZONE'), now()").Scan(&tz, &tm)
    if err != nil {
        log.Fatal(err)
    }
    log.Printf("current_setting('TIMEZONE'): %s, now(): %v\n", tz, tm)
}

returns

2024/04/27 21:31:38 current_setting('TIMEZONE'): UTC, now(): 2024-04-28 04:31:38.57887 +0000 UTC

This PR allows to override TZ for returned time.Time in code, simplifies migration from pq to pgx and doesn't break any existing code, which use pgx

Usage:

import (
...
    "github.com/jackc/pgtype"
...
)

func main() {
    pgtype.CustomTimeLocation = time.UTC
...

I saw several old topics about such problems (for example, https://stackoverflow.com/questions/72771272/how-to-setup-pgx-to-get-utc-values-from-db)

jackc commented 6 months ago

This functionality has just become available in pgx v5 in 8649231bb3bc00b4b9c180ce557a54ae41c28ce2 and 33360ab479cc29dd23adb848504bad01b7ff25b3. I'd prefer not to add a different approach to v4.

lansfy commented 6 months ago

@jackc , I read comments from specified commit and still has problem. For some reason I use third-party library dbr which opens database with sql.Open. It's not to easy to do

I think that this issue could be resolved by the following code in an after connect hook.

        conn.TypeMap().RegisterType(&pgtype.Type{
            Name:  "timestamptz",
            OID:   pgtype.TimestamptzOID,
            Codec: &pgtype.TimestamptzCodec{ScanLocation: time.UTC},
        })

because code doesn't have direct access to pgx.Connection

jackc commented 6 months ago

You can use RegisterConnConfig when you have to use sql.Open.

lansfy commented 6 months ago

@jackc , I did that, but AfterConnect has type func(ctx context.Context, conn *pgconn.PgConn) and as I understand pgconn.PgConn doesn't have TypeMap. And I still don't understand how to modify pgx.Conn


package main

import (
    "context"
    "fmt"
    "log"
    "time"

    "database/sql"
    "github.com/jackc/pgx/v5"
    "github.com/jackc/pgx/v5/pgconn"
    "github.com/jackc/pgx/v5/stdlib"
)

func main() {
    pcfg, err := pgx.ParseConfig("postgres://postgres:qwe123QWE@localhost:5432/postgres?sslmode=disable")
    if err != nil {
        log.Fatal(err)
    }

    pcfg.AfterConnect = func(ctx context.Context, conn *pgconn.PgConn) error {
        fmt.Println("?????") // ??????
        return nil
    }

    connStr := stdlib.RegisterConnConfig(pcfg)
    fmt.Println(connStr)

    db, err := sql.Open("pgx", connStr)
    if err != nil {
        log.Fatal(err)
    }

    var tz string
    var tm time.Time
    err = db.QueryRow("select current_setting('TIMEZONE'), now()").Scan(&tz, &tm)
    if err != nil {
        log.Fatal(err)
    }
    log.Printf("current_setting('TIMEZONE'): %s, now(): %v\n", tz, tm)
}
lansfy commented 6 months ago

Created similar pull-request to pgx

https://github.com/jackc/pgx/pull/2026

I think it will be useful to allow to redefine default timezone for database globally without such not-clear constructions.

jackc commented 6 months ago

I would prefer to avoid adding global variables. I also don't won't to make a special case for how to customize one particular type. The Codec system should be used for any type customization.

It is unfortunate the other library only supports sql.Open. If you could use stdlib.OpenDB then you could have an after connect hook that would do what you want.

If a change needs to be made to support libraries that don't support using existing an *sql.Db then I would say a RegisterConnConfigOptions would be a better direction. It would allow withing any OptionOpenDB.

fantapop commented 6 months ago

I think I'm in a similar boat here. buffalo/pop establishes the connection using sql.Open. Its buried under a few layers and doesn't provide any access to the connection being returned by sql.Open. https://github.com/gobuffalo/pop/blob/01ebd5b92a248a7870d2a4966b94282d59e19df4/connection_instrumented.go#L92