[pitch] SwiftNIO based PostgreSQL client

Yes, I think that would work well! There's one small concern I have: Let's imagine the app is written in a non-NIO style but it uses a library that is written in NIO. The app would then get a SimplePostgresConnection but the library might prefer a NIO-style PostgresConnection. For a database library (like the one you propose) all this is probably not that relevant because you typically don't hand database connections around to other libraries all that often but for other libraries that might be relevant.
But generally I think it would be cool to share the same type I think. And as you said, the non-NIO APIs are really just wrappers on top of the NIO-style API, that also why I created them mostly as extensions.

Let me know what you think.

That's a good point. The SimplePostgresConnection could (and probably should) expose its PostgresConnection as a public var, in case you need to convert for an API.

For this specific example, I wonder how likely it would be that someone using the non-NIO version of Postgres would need to interface with a NIO-version library? That seems like a complex enough case to merit just using NIO-based APIs for both libraries. I may be wrong here though.

My main concern with having the non-NIO methods directly alongside the NIO methods is that it adds (possibly confusing) bloat:

For NIO users:

  • There is a terminate() method that should be called but doesn't really do anything.
  • Auto-complete has extraneous methods which don't support futures.

For non-NIO users:

  • Auto-complete has extraneous methods which don't support callbacks.

I'm interested to hear your additional thoughts.

You can also have a separate package nio-postgres-dispatch (strawman name) that contains the wrappers, and hence if you don't import that you don't have those autocompletion issues when you just use the NIO flavored version at least.

Would be nice if there was a way to hide the NIO symbols if the library is just a dependency downstream and not top level.

I think having two structs one wrapping the other could be a good idea but maybe that naming is a bit NIO-centric, what about:

  • PostgresConnection: the standard Swift API wrapper
  • NIOPostgresConnection: the low-level NIO API (or even, nested PostgresConnection.NIO?)
1 Like

I think explicitly managing the lifecycle for scarce resources (ie. everything that isn't memory/CPU which are done automatically by ARC/the OS) is quite important and I would introduce it early in an API design. I don't think it's unreasonable for users to terminate Postgres. Therefore I would argue that in the NIO world, we should still have users call the terminate() method even if it might not do something.
In the future, we might want to implement some connection pooling and then the terminate() method could terminate the connection pool.

That's a fair point. We could in theory solve that with a module which holds the non-NIO extensions maybe?

Extra packages put (I think) too much burden on the package maintainers but a separate module might do it.

Maybe we could have a NIOPostgres module which gives you all the NIO APIs only and another module Postgres which does @_exported import NIOPostgres and therefore would give you both styles of API. Then most people would just import Postgres and get both but if you really only want the NIO stuff, you'd do import NIOPostgres.

2 Likes

Absolutely! That's a lot better. I completely forgot about @_exported!

Having a basic NIO PostgreSQL lib would be great, I also considered approaching this. There are essentially three implementations I know of: the Vapor one, then there was another GCD(?) based one on GitHub from which I don't remember the name (was the Vapor one derived from that?), and finally I also have an (unreleased) partial protocol implementation for Noze.io. I think it is fair to say that the Vapor one is tested best.

What I very much dislike about the Vapor implementation is that it tightly couples with Codable, I wouldn't want that - at least not at this level.

I would love to see un-opinionated PostgreSQL decoder/encoder channels in the spirit of NIOIRC or NIORedis (or libpq for that matter). They would yield raw PostgreSQL packets w/o copying of data.
For example (if I remember right) the PG protocol returns rows as a single framed packet. Lets assume a NIO channel receives a ByteBuffer. Lets say that buffer is 500 bytes and contains like 80 pkey rows.
What I'd like the decoder to read-emit is a small PostgreSQLRowPacket struct which refers to the ByteBuffer slice for that row, and provides ways to extract the desired base types by index (and potentially column name, I think PG also transmits a row schema which could be shared between those rows).

Arbitrary Coding approaches could then efficiently built on top of this (and likely once for different DB client libs). I guess it would be nice to have a basic "batteries included" builtin client library, but that is Step 2 IMO.

That also again outlines the potential need for two things which are not specific to PG at all but affect pretty much any client library:

  • Connection Setup Strategies (retries, etc)
  • Connection Pooling

It would be really good to have standard NIO implementations for this.

There's a fundamental difference between NIO users and non-NIO users. ... event loops ...

IMO the approach in my IRC client is reasonable. Simple and deals with both scenarios in reasonable ways.

2 Likes

I'm probably missing something here, but in what way does the current implementation not provide these capabilities?

Queries provide a PostgresRow, which in turn have methods to access underlying PostgresData that have the fundamental byteBuffer property.

I was following Tanner's pitch and started working on a proposal for a SwiftNIO Redis client, and after working on that I've come to the same conclusion. Perhaps we should have another pitch for a "Database Driver API" library, akin to the Logging proposal?

Hm, fair point. Looks like PostgresMessage.InboundHandler is pretty close to what I'm looking for!
Can we just have that and put the Codable stuff in a different module? Just a clean module yielding and taking those packets?

(BTW: I think the PostgresMessage enum might be a little big to be passed around in NIOAny's, @johannesweiss ?)

Perhaps we should have another pitch for a "Database Driver API" library

I don't think this is database specific at all either, it is also the same for IRC or IMAP4 or HTTP, in short: essentially any client.

A "database driver API" might be the thing which holds the types to make database protocol libs do the Codable thing.

1 Like

Although unfortunate I think it may be worthwhile to have separate NIOPostgresClient and NIOPostgresClientTLS modules.

Why? The basic NIOPostgres package would be just the raw decoder and essentially a really tiny module doing the hard work. Even a NIOPostgresClient module would be rather small, especially if we can get pooling and connection setup into a more general module.

It would be a really a waste having to pull down the whole nio-ssl just for that. Particularly because many people won't connect to PG using TLS.
(and many (presumably almost all) projects are running a proxy for frontend TLS, so they might not link nio-ssl at all)

This sounds similar to Vapor's DatabaseKit package. That includes some core Database fundamentals like a ConnectionPool.

I agree. The plan is that Vapor's higher level PostgresKit package will combine NIOPostgres and DatabaseKit to create a high-level, easy to use Postgres client.

It should be possible to move the Codable stuff to that higher layer. I put it in NIOPostgres originally because it allows for the details of PostgresMessage to be kept internal. But it sounds like we might want to make that public anyway.

If you want, I could have a look on isolating the pure protocol parts (similar to what I did with the very plain NIOIRC vs the more opinionated IRC Client module).
If that's not being done here, I would probably do it for my own purposes anyways :slight_smile:

1 Like

I see PostgresKit being the right place to put the Codable stuff. I do think NIOPostgres should still take care of converting PostgresData to Swift types like String and Int, but that can we done with a simple protocol like PostgresDataConvertible. Then, in PostgresKit, Coders can be built on top on top of the convertible protocol.

I will take a stab at this soon (hopefully this week), but I am always happy for PRs :)

Now that the Logging API has been accepted into Sandbox, with Metrics coming soon, we should probably discuss how and what we want to log.

1 Like

To provide my perspective on logging, basing them off of the discussion on log levels between os_log and the logging framework:

The driver should only log to debug, or critical and higher.

Is there any way to distinguish verbosity of debug levels? For example, I'd love to have an extra verbose debug level (think -vvv) where details about all incoming / outgoing postgres messages and how they are being parsed is printed. This would be helpful during development and tracking down bugs.

Besides that, I think replacing any existence of print("[ERROR] ...") with log.error(...) will be pretty straightforward.

The current logging API does define trace above debug

That seems appropriate for most of the "debug" logging we might want to do in the driver

1 Like

Note however that there's a new thread discussing the log levels that both os_log and the server logging API are going to use. So it looks like it'll become syslog levels (and in fact in the repo that is very soon to be opened I made that change).