Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is somehow possible to import multiple versions? #1480

Closed
prochac opened this issue Jan 27, 2023 · 4 comments · Fixed by #1498
Closed

Is somehow possible to import multiple versions? #1480

prochac opened this issue Jan 27, 2023 · 4 comments · Fixed by #1498
Labels

Comments

@prochac
Copy link
Contributor

prochac commented Jan 27, 2023

Describe the bug
If two pgx/stdlib versions are imported, it leads to panic.

To Reproduce
Steps to reproduce the behavior:

package main

import (
        "fmt"

        _ "github.com/jackc/pgx/v4/stdlib"
        _ "github.com/jackc/pgx/v5/stdlib"
)

func main() {
        fmt.Println("Hello, 世界")
}

Expected behavior
Could import both at the same time, when Go modules allow it thanks to version suffix /v2.

Actual behavior
Panics.
It doesn't allow A/B release behind feature flag.

panic: sql: Register called twice for driver pgx

goroutine 1 [running]:
database/sql.Register({0x56796b, 0x3}, {0x5af900, 0xc0000a2228})
        /usr/lib/go/src/database/sql/sql.go:51 +0x13d
github.com/jackc/pgx/v5/stdlib.init.0()
        /home/prochac/go/pkg/mod/github.com/jackc/pgx/v5@v5.2.0/stdlib/sql.go:88 +0x85
exit status 2

Version
go version go1.19.5 linux/amd64

@prochac prochac added the bug label Jan 27, 2023
@prochac
Copy link
Contributor Author

prochac commented Jan 27, 2023

We can't unregister driver, but we can check if already registered
https://pkg.go.dev/database/sql#Drivers

The drivers could be registered with the major vesion in mind
"pgx/v4" and "pgx/v5" instead of "pgx"
The first imported could register "pgx" too. <-- this will keep backward compatibility

In my example, it could be

import (
        _ "github.com/jackc/pgx/v4/stdlib"
        _ "github.com/jackc/pgx/v5/stdlib"
)

sql.Open("pgx", "...") // v4
sql.Open("pgx/v4", "...") // v4
sql.Open("pgx/v5", "...") // v5

alternative, that can collide with goimports tool or others, though

import (
        _ "github.com/jackc/pgx/v5/stdlib"
        _ "github.com/jackc/pgx/v4/stdlib"
)

sql.Open("pgx", "...") // v5
sql.Open("pgx/v4", "...") // v4
sql.Open("pgx/v5", "...") // v5

Other way to explicitly choose the "default" driver

var _ = func() (_ bool) {
        sql.Register("pgx", &stdlib.Driver{})
        return
}

func main() {
        fmt.Println("Hello, 世界")
}

@jackc
Copy link
Owner

jackc commented Jan 27, 2023

The drivers could be registered with the major vesion in mind
"pgx/v4" and "pgx/v5" instead of "pgx"
The first imported could register "pgx" too. <-- this will keep backward compatibility

I like this idea.

@prochac
Copy link
Contributor Author

prochac commented Feb 7, 2023

I will check if my premise is valid in some free time. I'm a bit worried about how the init funcs imports are implemented, and if they concurrent safe*.
If it will be ok, I would prepare PR, it should be quite simple :)

*) the safety is a crucial condition for calling sql.Drivers(). I'm a bit nervous about the internal mutex
https://cs.opensource.google/go/go/+/refs/tags/go1.20:src/database/sql/sql.go;drc=b46e44a399045d0177dd063dc192168f0b5b3f55;l=34

@jackc
Copy link
Owner

jackc commented Feb 7, 2023

I expect it should be fine. From the Go spec (https://go.dev/ref/spec#Program_initialization_and_execution):

Package initialization—variable initialization and the invocation of init functions—happens in a single goroutine, sequentially, one package at a time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants