Add QueryRewriter interface (and named arguments)

[jackc]

A long-standing request is to support named arguments. e.g.

conn.Exec(ctx,
  `insert into users (id, email, ...) values (:id, :email, ....);`,
  map[string]any{"id": 42, "email": "foo@example.com", ...},
)

// Instead of

conn.Exec(ctx,
  `insert into users (id, email, ...) values ($1, $2, ...);`,
  42, "foo@example.com", ...,
)

The actual PostgreSQL $1 style placeholders are not bad when there are only a few. But it rapidly gets difficult to manage when there are a lot of them especially when certain arguments are used multiple times.

I propose adding QueryRewriter interface defined something like:

type QueryRewriter struct {
  RewriteQuery(ctx context.Context, conn *pgx.Conn, sql string, args ...any) (newSQL, string, newArgs []any)
}

If a QueryRewriter was the first argument to a query its RewriteQuery method would be called and the query would actually run on the results.

This would provide an means to add a type NamedArgs map[string]any that implements named arguments via QueryRewriter. In this way named arguments could be added fairly seamlessly.

In addition, QueryRewriter might be useful for a number of other utilities. e.g. an InsertArgs that is used to create the column list and the values clause of an insert statement. Not sure if anything past NamedArgs would actually be useful or a good idea but the flexibility of this approach would enable all sorts of experiments like that without needing to put them in pgx.

[jackc]

Added QueryRewriter mostly as described above and added NamedArgs.

Example usage:

conn.Query(ctx, "select * from widgets where foo = @foo and bar = @bar", pgx.NamedArgs{"foo": 1, "bar": 2}))

[tsingson]

Added QueryRewriter mostly as described above and added NamedArgs.

Example usage:

conn.Query(ctx, "select * from widgets where foo = @foo and bar = @bar", pgx.NamedArgs{"foo": 1, "bar": 2}))

should use in prepare sql statement ??

[jackc]

This wouldn't work with explicit prepared statements. But with pgx automatically prepares and caches statements so explicit prepared statements generally shouldn't be used.

[mwchambers]

Hi @jackc
.
First of all thank you for all your work on pgx. The new pgx.Collect* and pgx.RowTo* are really nice. Thanks!

I have just been looking at NamedArgs and wondered what would happen if there was a typo meaning a mismatch between the placeholders in the SQL and the map keys of NamedArgs. I was hoping I would get an error in the second example below.

Is this expected behaviour, or a oversight?

First I get 323 rows, as expected:

	// This works as expected.
	rows, _ := conn.Query(ctx, `
		SELECT COUNT(*)
		FROM cron
		WHERE school_id = @school_id
		`,
		pgx.NamedArgs{"school_id": 2})

	total, err := pgx.CollectOneRow(rows, pgx.RowTo[int32])
	is.NoErr(err)
	t.Logf("Got %d rows", total) // Got 323 rows

Now check if we get an error if we mismatch the NamedArgs map key with the placeholder.

	rows, _ = conn.Query(ctx, `
		SELECT COUNT(*)
		FROM cron
		WHERE school_id = @school_id`,
		pgx.NamedArgs{"school_iD": 2}) // The key does not match the placeholder

	total, err = pgx.CollectOneRow(rows, pgx.RowTo[int32])
	is.NoErr(err)
	t.Logf("Got %d rows", total) // Got 0 rows

No error, no rows.

[jackc]

@mwchambers

First of all thank you for all your work on pgx. The new pgx.Collect* and pgx.RowTo* are really nice. Thanks!

You're welcome!

I have just been looking at NamedArgs and wondered what would happen if there was a typo meaning a mismatch between the placeholders in the SQL and the map keys of NamedArgs. I was hoping I would get an error in the second example below.

Is this expected behaviour, or a oversight?

It's expected, though I can see the argument against it.

There are two cases:

1. The query references a key that the NamedArgs doesn't contain.
2. The NamedArgs contains a key that is not used.

Both cases could be an error. But both cases can also be convenient. 1. allows optional fields that default to NULL. 2. automatically filters extra values when the original map came from an untrusted source.

I suppose a StrictNamedArgs could be added to allow either behavior to be used.

[mwchambers]

@jackc Thanks for your feedback.

Both cases could be an error. But both cases can also be convenient. 1. allows optional fields that default to NULL. 2. automatically filters extra values when the original map came from an untrusted source.

I suppose a StrictNamedArgs could be added to allow either behavior to be used.

I would prefer it be an error unless the map keys are exactly equal to the placeholders.

I took a look at implementing your StrictNamedArgs idea but I cannot see that it will be possible without changing the QueryRewriter interface. Can I communicate error back?

if queryRewriter != nil {
	sql, args = queryRewriter.RewriteQuery(ctx, c, sql, args)
}

Would you consider changing the QueryRewriter interface so that RewriteQuery returns an error which would be returned on Conn.Query() / Rows.Err() allowing for QueryRewriter implementations to report an error?

[jackc]

Would you consider changing the QueryRewriter interface so that RewriteQuery returns an error which would be returned on Conn.Query() / Rows.Err() allowing for QueryRewriter implementations to report an error?

Technically that would be a breaking change. But given that v5 is so new and this is likely not been used by external code I think that breaking semver would be justifiable here.

[jackc]

@mwchambers I added the error return to RewriteQuery in 7d3b9c1.

[mwchambers]

@jackc Thanks for this!

This is really quite useful. I can now make any custom 'sql builder' type implement pgx.QueryRewriter and pass the SQL builder instance directly to pgx with the ability for the builder to return an error. As an example:

// sqlb would be similar to your pgsql package,
// but returns one Query type which implements pgx.QueryRewriter
// It sets an error if the placeholders to not match 1:1
query := sqlb.New("SELECT foo, bar FROM baz WHERE x = ? AND y = ?", 1, 2)

// No need for pgsql.Build() or similar to get sql and args
rows, _ := conn.Query(ctx, "sql param is not used", query)

// Or...
query := sqlb.Insert("widget", sqlb.RowMap{"name": "Mark", "total": 42})
_, err = conn.Exec(ctx, "", query)

I have decided against using NamedArgs / StrictNamedArgs after applying it to parts of some existing code to see how it worked out. It would add a lot of repetition of the column names and not add much benefit. I thought it would be useful when editing params in the middle of large query so you don't have to realign $3, $4 etc in the SQL, keeping the diff a lot cleaner. Your pgsql ? approach solves this issue without having to name all the columns (though perhaps not as safe as no sqlLexer)

So again, thank you. This gives us lots of flexibility for experimentation.