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

Allow using database/sql driver with functioning pgx listener #352

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bgentry
Copy link
Contributor

@bgentry bgentry commented May 14, 2024

Building on #351, this PR allows users to take advantage of the new riverdatabasesql driver while also keeping full support for LISTEN and avoiding poll-only mode.

This is accomplished through a riverdatabasesql.NewWithListener() constructor that allows the database/sql driver to be used with a functioning listener implementation. Also adds a riverpgxv5.NewListener() constructor to allow creating a listener with a raw pgx pool.

These can be combined to allow full listener support as long as the underlying database driver supports it, even when it's used within an abstraction like database/sql or Bun.

Needs tests if we want to proceed with this. I'm pleased with how little code it took to achieve though!

You could imagine this being used in the future with a non-Postgres listener implementation such as Redis, though for that to work we may need to loosen notifications to be emitted outside of a transaction (and may not be able to emit them automatically at all for the *Tx insert variants bc we cannot tell when a transaction completes).

Here, implement the rest of driver functionality on `riverdatabasesql`,
the existing driver for Go's built-in `database/sql` package. Previously
it only supported a minimal interface allowing it to run migrations, but
nothing more sophisticated like inserting jobs.

The benefit of a fully functional driver is that it will allow River to
be integrated with with other Go database packages that aren't built
around Pgx like Bun (requested in #302) and GORM (requested in #58).
I'll need to write up some documentation, but this change should make
both of those integrations possible immediately.

It also lays the groundwork for future non-Postgres drivers. It's going
to be a little more still, but I want to take a stab at SQLite, and this
change will get us a lot of the way there.

There's no way with `database/sql` to support listen/notify, so here we
introduce the idea of a poll only driver. River's client checks whether
a driver can support listen/notify on initialization, and if not, it
enters poll only mode the same way as if configured with `PollOnly`.

An intuitive idiosyncrasy of this set up is that even when using the
`database/sql` driver bundled here, regardless of whether they're
working with Bun, GORM, or whatever,  users will generally still be
using Pgx under the hood since it's the only maintained and fully
functional Postgres driver in the Go ecosystem. With that said, the
driver still has to bundle in `lib/pq` for various constructs like
`pq.Array` because we're using sqlc, and sqlc's `database/sql` driver
always uses `lib/pq`. I tried to find a way around this, but came out
fairly convinced that there is none. To rid ourselves of `lib/pq`
completely we'd need sqlc to ship an alternative Pgx driver that used
Pgx internally, but exposed a `database/sql` interface using `*sql.Tx`
instead of `pgx.Tx`.
Add a `NewWithListener` constructor to `riverdatabasesql` that allows
the `database/sql` driver to be used with a functioning listener
implementation. Also add a `NewListener` constructor to the `riverpgxv5`
driver to allow creating a listener with a raw pgx pool.

These can be combined to allow full listener support as long as the
underlying database driver supports it, even when it's used within an
abstraction like `database/sql` or Bun.
@brandur brandur force-pushed the brandur-functional-database-sql branch 3 times, most recently from 7c7bac8 to 01aeece Compare July 6, 2024 00:51
Base automatically changed from brandur-functional-database-sql to master July 6, 2024 01:00
@bgentry bgentry mentioned this pull request Aug 14, 2024
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