jschaf / pggen

Generate type-safe Go for any Postgres query. If Postgres can run the query, pggen can generate code for it.
MIT License
282 stars 26 forks source link

Working with Postgres extensions #9

Closed td0m closed 3 years ago

td0m commented 3 years ago

This might be a long shot, but will it be possible to use pggen with Postgres extensions?

Currently, it does not seem to work. For example, given the following schema:

CREATE EXTENSION IF NOT EXISTS pgcrypto;
CREATE TABLE "user"(
  email TEXT PRIMARY KEY,
  pass TEXT NOT NULL
);

And query:

-- name: CreateUser :one
INSERT INTO "user"(email, pass)
VALUES (pggen.arg('email'), crypto(pggen.arg('password'), gen_salt('bf')));

The following error appears:

infer typed named query CreateUser: infer input types for query:
exec prepare statement to infer input query types:
ERROR: function crypto(unknown, text) does not exist (SQLSTATE 42883)
td0m commented 3 years ago

Same goes for custom types, for example:

CREATE EXTENSION IF NOT EXISTS postgis;

CREATE TYPE visit(
  -- ... fields omitted
  geo GEOGRAPHY
);
--- name: Visit :exec
INSERT INTO visit(geo)
VALUES(pggen.arg('geo'));

We get:

no go type found for Postgres type geography oid=17912
jschaf commented 3 years ago

This might be a long shot, but will it be possible to use pggen with Postgres extensions?

Totally! It should "just work" right now. pggen essentially treats Postgres as a black box. You can tweak the database however you want, pggen only needs the input and output types.

ERROR: function crypto(unknown, text) does not exist (SQLSTATE 42883)

This error indicates something is wrong with the setup. For this particular case, the function name is crypt, not crypto. I added an example in https://github.com/jschaf/pggen/commit/1564c8d1202d658e8c2dab5cb0e7ce638a72f92c to make sure it works. Just change crypto to crypt and it should work.

Also, you'll need to change :one to :exec or pggen will complain that the query can't possibly return a row.

no go type found for Postgres type geography oid=17912

tl;dr: You need to create a Go type Geography and tell pggen about the mapping with --go-type geography=github.com/mytype.Geography. See https://github.com/jschaf/pggen/pull/10 for a mostly complete example.

The problem is pggen doesn't know what Go type to use for geography. The library pgtype doesn't include Geography, so as far as I know, there's no existing Go types for postgis. Geography is a base type in pg_type, so no information about child types is exposed so there's not much pggen can do. We could hardcode these types, but then we'd have to include the type definition in every generated package.

The right solution is to implement these types in pgtype. The maintainer is pretty responsive if you want to go that route. If you can get them into pgtype, I think I'd be okay adding the default type mappings by type name into pggen. I'll have to think how that could break things; it's probably safe if we check that the type name was created by the right extension.

I added a mostly working example in https://github.com/jschaf/pggen/pull/10. The only thing missing is a Go type for geography.

As a somewhat interesting aside about OIDs Right now, pggen registers a bunch of type mappings by OID. We can only use OIDs for builtin Postgres types because OIDs are assigned through an auto-incremented number. So the OIDs for geography will be different if you switch the orders of the create extension statements below: ``` CREATE EXTENSION postgis; CREATE EXTENSION pgcrypto; ``` Long term, I could register type mapping by name. That's worth doing for all of the default extensions, like `pgcrypto` and `postgis` if there's an existing type in `pgtype`.
jschaf commented 3 years ago

Closing because pggen works with extensions. The postgis extension is more complicated because there's not Go type in pgtype for geography and friends.

Feel free to follow-up but I don't think there's anything to implement right now in pggen.

jschaf commented 3 years ago

One other idea I had was to allow specifying the docker image that pggen uses instead of hard coding the Postgres:13 image. That allows a lot more flexibility to use pggen without spinning up your own docker image.

The downside is it requires another flag, maybe —docker-image. The flag is probably worth it.

Another downside is it’s pretty easy to break a docker image and right now it’s not clear if pggen failed or Postgres failed.

If I add the docker flag, I probably need to allow custom connection strings with Docker since folks will have custom database names and users. Right now, pggen assumes hardcoded user name and database name when connecting to docker Postgres.