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

NamedStmt.QueryRowContext() doesn't cancel query when context.DeadlineExceeded has occurred #745

Closed
michaelshobbs opened this issue Jun 10, 2021 · 4 comments

Comments

@michaelshobbs
Copy link

michaelshobbs commented Jun 10, 2021

I'm not certain if this is a bug (here or database/sql) or just me misusing this library. In any case, I'd like to call QueryRowContext() and have it cancel the query when the context Deadline has been exceeded. This seems to not happen.

go version: go version go1.15.10 darwin/amd64
sqlx version: v1.3.4

Example test case:

func TestContextDeadlineExceeded(t *testing.T) {
	queryCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
	defer cancel()

	type Var1 struct {
		ID int `db:"id"`
	}

	db, err := Connect("postgres", os.Getenv("SQLX_POSTGRES_DSN"))
	if err != nil {
		t.Fatal(err)
	}
	var d Var1
	fmt.Println("calling PrepareNamedContext")
	stmt, err := db.PrepareNamedContext(queryCtx, `SELECT pg_sleep(10) AS id`)
	if err != nil {
		t.Fatal(err)
	}
	fmt.Println("calling QueryRowContext")
	err = stmt.QueryRowContext(queryCtx, &d).Err()
	if err == nil || !errors.Is(err, context.DeadlineExceeded) {
		fmt.Printf("queryCtx.Err(): %+v\n", queryCtx.Err())
		t.Fatalf("err: %v expected: %T", err, context.DeadlineExceeded)
	}
}

Any guidance would be appreciated.

@michaelshobbs michaelshobbs changed the title NamedStmt.QueryRowContext() doesn't error with context.DeadlineExceeded NamedStmt.QueryRowContext() doesn't cancel query when context.DeadlineExceeded has occurred Jun 10, 2021
@michaelshobbs
Copy link
Author

michaelshobbs commented Jun 10, 2021

I was able to recreate this in the std lib. Does anyone know why the stmt doesn't get canceled like with db.QueryRowxContext?

To clarify, context.DeadlineExceeded is returned when calling row.Scan() OR rowx.StructScan(). However, the call to stmt.QueryRowContext is not canceled until the query returns from the database

example of this behavior

func TestSQLContextDeadlineExceeded(t *testing.T) {
	queryCtx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
	defer cancel()

	db, err := Connect("postgres", os.Getenv("SQLX_POSTGRES_DSN"))
	if err != nil {
		t.Fatal(err)
	}
	stmt, err := db.DB.PrepareContext(queryCtx, `SELECT pg_sleep(10) AS id`)
	if err != nil {
		t.Fatal(err)
	}
	type Var1 struct {
		ID int `db:"id"`
	}
	var d Var1

	go func() {
		time.Sleep(5 * time.Second)
		dl, ok := queryCtx.Deadline()
		if ok && time.Now().After(dl) {
			panic(context.DeadlineExceeded)
		}
	}()

	err = stmt.QueryRowContext(queryCtx).Scan(&d)
	if err == nil || !errors.Is(err, context.DeadlineExceeded) {
		fmt.Printf("queryCtx.Err(): %+v\n", queryCtx.Err())
		t.Fatalf("err: %v expected: %T", err, context.DeadlineExceeded)
	}
}

@michaelshobbs
Copy link
Author

This actually may be an issue in github.com/lib/pq

@michaelshobbs
Copy link
Author

michaelshobbs commented Jun 11, 2021

This definitely seems to be in github.com/lib/pq. The following test case passes using mysql:

func TestSQLContextDeadlineExceeded_mysql(t *testing.T) {
	queryCtx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
	defer cancel()

	db, err := Connect("mysql", os.Getenv("SQLX_MYSQL_DSN"))
	if err != nil {
		t.Fatal(err)
	}
	stmt, err := db.DB.PrepareContext(queryCtx, `SELECT sleep(10) AS id`)
	if err != nil {
		t.Fatal(err)
	}
	type Var1 struct {
		ID int `db:"id"`
	}
	var d Var1

	go func() {
		time.Sleep(5 * time.Second)
		dl, ok := queryCtx.Deadline()
		if ok && time.Now().After(dl) {
			panic(context.DeadlineExceeded)
		}
	}()
	err = stmt.QueryRowContext(queryCtx).Scan(&d)
	if err == nil || !errors.Is(err, context.DeadlineExceeded) {
		fmt.Printf("queryCtx.Err(): %+v\n", queryCtx.Err())
		t.Fatalf("err[%T]: %v expected: %T", err, err, context.DeadlineExceeded)
	}
}

I'll comment back here once I figure out the root cause. I'll probably have to switch to https://github.com/jackc/pgx anyway since lib/pq has gone into maintenance mode.

@michaelshobbs
Copy link
Author

Found it! Turns out lib/pq didn't implement the interfaces. Here is the PR for folks that run into this down the road.

lib/pq#1047

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

No branches or pull requests

1 participant