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

feat: add dedicated method to extract error from errRow #2154

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aerialls
Copy link

Add a dedicated method to extract error from errRow. This is already present for errRows. This allows the use case to extract the error to retry if necessary.

type errRow interface {
	pgx.Row
	Err() error
}

func (r *RetriableDBTX) QueryRow(ctx context.Context, sql string, args ...any) pgx.Row {
	row := r.dbtx.QueryRow(ctx, sql, args...)
	if er, ok := row.(errRow); ok {
		// Custom logic to retry
	}

	return row
}

@jackc
Copy link
Owner

jackc commented Oct 23, 2024

I'm not sure I understand the use case. In your example I would expect you to call Scan and handle the returned error.

The reason errRows has an Err() method is to satisfy the Rows interface. And the reason errRow doesn't is because the Row interface doesn't.

@aerialls
Copy link
Author

aerialls commented Oct 24, 2024

I'm using sqlc and trying to add a global retry mechanism for a read-only connection. sqlc is using the following interface as an entrypoint.

type DBTX interface {
	Exec(context.Context, string, ...interface{}) (pgconn.CommandTag, error)
	Query(context.Context, string, ...interface{}) (pgx.Rows, error)
	QueryRow(context.Context, string, ...interface{}) pgx.Row
}

My initial idea was to create a RetriableDBTX with the same interface as an entry-point for sqlc and to retry the method in case of an error. It's working for Exec and Query but I don't have any error for QueryRow and so my idea to be able to retrieve the error if we have an Err() error method. The Scan method is executed afterward by sqlc for each SQL query so it's much harder to perform the retry logic here.

@jackc
Copy link
Owner

jackc commented Oct 25, 2024

This change won't do what you want. And what you are doing with Query() isn't reliable either.

Query() will only return an error in limited circumstances such as a network failure sending the request. The returned Rows has to be closed and Err() checked to know if it successfully ran.

QueryRow() is a very simple wrapper around Query(). The result isn't actually read until Scan() is called.

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

Successfully merging this pull request may close these issues.

2 participants