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

pgx / pgxpool Interceptors aka Middleware (for caching, circuit breakers, etc.) #1935

Closed
StevenACoffman opened this issue Mar 9, 2024 · 8 comments

Comments

@StevenACoffman
Copy link

StevenACoffman commented Mar 9, 2024

Problem Statement: Generic Interceptor (e.g. Caching)

The pgx interface is faster than the database/sql interface. The pgx interface is faster. Many PostgreSQL-specific features such as LISTEN / NOTIFY and COPY are not available through the database/sql interface.

However, ngrok/sqlmw provides database/sql interface interceptors (middleware).

This has facilitated use cases like prashanthpai/sqlcache which provides a nice declarative mechanism for caching individual queries to improve performance. Other use cases like circuit breakers, retry mechanisms, etc. are also facilitated by having generic interceptor interfaces.

Specifically, I want to write an interceptor using the pgx / pgxpool interface that would provide for sqlcache, so that when the cache misses it would still use the faster pgx interface, and allow PostgreSQL-specific features.

Describe the solution you'd like
A package like ngrok/sqlmw specific to pgx / pgxpool to facilitate a generic caching layer like prashanthpai/sqlcache without resorting to the database/sql interface.

prashanthpai/sqlcache relies on query "annotations", like so:

-- name: GetUsers :many
-- @cache-ttl 30
-- @cache-max-rows 10
SELECT * FROM users;
@jackc
Copy link
Owner

jackc commented Mar 11, 2024

I'm open to additional extension points, but the proposal would need to be more specific.

There are already several extensions points that overlap with a generic middleware, the tracing interfaces and the QueryRewriter interface in particular. Any new system would need to play nice with the existing architecture.

In addition, it should be something that has significant advantages over a wrapper or container. For example, with caching in particular, I would usually assume that caching would be more effective at a higher layer. That is, the final domain objects are cached, not the low-level row data that needs to be reparsed.

@StevenACoffman
Copy link
Author

That's actually two great points.

Moving caching/retry/circuit breaker decisions outside of the driver (e.g. sqlc generated Go code) would perform even better by avoiding reparsing of the SQL.

Also, it's probably a better pattern to make cache domain objects.

@atombender
Copy link
Contributor

@jackc I have a use case that I think requires more extension points:

I'd like to automatically decorate all executed SQL statements with a header comment to make it easier for developers and administrators to identify the source of a query when looking at logs or pg_stat_activity. This comment includes contextual informational, such as an application-level "query name", API route, security principal, and similar things.

A decorated query might look like this:

-- app="FizzBuzz" name="DeleteUser" route="/v:version/users/delete/:id"
DELETE FROM users WHERE ... etc.

The caller can provide this information out of band by using some functions that add values to context.Context.

At the moment, I accomplish this explicitly by wrapping a transaction:

func TxWithTagging(tx pgx.Tx) pgx.Tx {
	return &taggingTx{
		Tx: tx,
	}
}

type taggingTx struct {
	pgx.Tx
}

func (t taggingTx) Exec(ctx context.Context, sql string, args ...any) (commandTag pgconn.CommandTag, err error) {
	sql = decorateSQL(ctx, sql)
	return t.Tx.Exec(ctx, sql, args...)
}
// ...

However, *pgx.Conn is not an interface and cannot be wrapped the same way, so this has to be done at the application level.

As far as I can see, this can't be accomplished today using the trace interface, and I also can't use the QueryRewriter, because it's supposed to be transparent to the caller. So an interceptor-type interface would be really useful.

@StevenACoffman
Copy link
Author

@atombender You might look at how SQL Commenter was implemented for the SQL driver (and how people were hoping to add it to otelpgx)

@atombender
Copy link
Contributor

@StevenACoffman They don't seem to have been successful at doing this with Pgx, though. I can see that using SQL Commenter through sql.Driver works, but I use Pgx directly.

@StevenACoffman
Copy link
Author

Sorry, I was unclear.

The SQL Commenter use case is almost identical to what you mentioned, but the fact that it is still unimplemented in exaring/otelpgx is a notable gap in the pgx ecosystem compared to the SQL driver ecosystem.

I was mentioning it more because it helps to build support for @jackc accepting a PR, not because it helps you implement one. :)

@atombender
Copy link
Contributor

Makes sense, agreed!

@jackc
Copy link
Owner

jackc commented Mar 22, 2025

I was mentioning it more because it helps to build support for @jackc accepting a PR, not because it helps you implement one.

As I mentioned above, I'm not against the idea. But that's all it is at this point. There hasn't been any interface proposed or PR submitted.


One additional concern I would have is that this could have a negative impact on performance. In particular, adding something like a request ID to the SQL text as a comment would make the prepared statement cache counterproductive. You would have to fall back to an exec mode that took two round trips or forego the binary format.

An ideal interface for this idea would have some way of working with QueryExecModeCacheDescribe such that the annotated query was sent to PostgreSQL but the unannotated query was used to find the query description.

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

3 participants