Is `pgcrypto` necessary?

[dmolesUC]

We're working on adding GoodJob to an app and recently had a deploy failure because the GoodJob migrations wouldn't run (on PG 12.3) without superuser privileges:

ActiveRecord::StatementInvalid: PG::InsufficientPrivilege: ERROR:  permission denied to create extension "pgcrypto"
HINT:  Must be superuser to create this extension.
/opt/app/db/migrate/20221208193908_create_good_jobs.rb:4:in `change'
/opt/app/bin/rails:4:in `require'
/opt/app/bin/rails:4:in `<main>'

For the life of me, though, I can't figure out what pgcrypto features GoodJob actually uses — I've tried grepping the source for all the functions I can find in the PG 12 pgcrypto docs and can't find any references to them:

digest
hmac
crypt
gen_salt
pgp_sym_encrypt
pgp_sym_decrypt
pgp_pub_encrypt
pgp_pub_decrypt
pgp_key_id
armor

Is pgcrypto actually used? And if so, for what?

[bensheldon]

The pgcrypto extension is used for generating UUID primary keys. I'm fairly certain it is a necessity. You could try removing the add_extension from the migration in development and see what happens.

[dmolesUC]

@bensheldon OK. It looks like in PG 12, gen_random_uuid() is in pgcrypto; in PG 13+ gen_random_uuid() is built in. (I didn't catch it in the source b/c my quick and dirty search for functions in the docs was only catching the ones with their own section headers, my bad.) Since it seems like in PG 13+ you don't need superuser privileges to install pgcrypto, it probably doesn't do any harm to leave it in (at least for us, since we can elevate privileges before we run that migration).

[bensheldon]

in PG 13+ gen_random_uuid() is built in.

Yay! Thanks for sharing that research. I hope generating a UUID is part of every tool's standard library 😄

I wonder if we should add a comment in the migration that's like # not required for Postgres v13+

[dmolesUC]

That sounds like a good idea. I wonder how hard it is to detect the Postgres version at migration time?

[bensheldon]

I wonder how hard it is to detect the Postgres version at migration time?

It's pretty easy to do a Postgres Version check:

good_job/app/models/good_job/lockable.rb

Line 231 in a559649

@_supports_cte_materialization_specifiers = connection.postgresql_version >= 120000

...but I don't generally think migrations should be very dynamic. Within Good Job, I treat migrations as a template for the application developer. But providing helpful hints is 👍🏻

[bensheldon]

I made this PR to help: #837

[dmolesUC]

That makes sense. Maybe a note in the relevant section of the docs, just to make it clear that's the philosophy? IDK what exactly, something like "Review the generated migrations and make adjustments for your application (if needed; in most cases the generated migrations will work out of the box)." (That's a little awkward, but you get the idea.)

(I mean, ideally everybody would do that as a matter of course, but novice developers may not think to or have the confidence to; and some library developers would be like: "oh, well, if you edited my migrations, don't expect any help with your issues".)

[bensheldon]

I changed the language and made it more action-oriented with it commented-out by default

Uncomment for Postgres v12 or earlier to enable gen_random_uuid() support