Squarespace / pgbedrock

Manage a Postgres cluster's roles, role memberships, schema ownership, and privileges
https://pgbedrock.readthedocs.io/en/latest/
Other
311 stars 35 forks source link

Select explicit fields from pg_authid #29

Closed zcmarine closed 6 years ago

zcmarine commented 6 years ago

Previously we selected * from pg_authid, which had a few negative effects.

First, this is vague and sloppy: we should select what we want, especially as the fields in pg_authid differ across Postgres versions.

Second, because the fields in pg_authid differ across Postgres versions, this introduces a variety of opaque bugs for users trying to get pgbedrock working with Postgres versions that aren't explicitly supported. If a field is missing from that version of pg_authid then a KeyError or something similar will come up further in the innards of pgbedrock, but the issue was much earlier in the results from the SELECT. If an extra (unexpected) field is in pg_authid then pgbedrock may fail as well. In both situations, the solution is to be explicit about what we request from the database and to fail on that query.

zcmarine commented 6 years ago

Hmm interesting that this Travis CI fails as make test runs fine locally. I'm going to decline this PR until PR #28 is merged as that one makes Travis CI's testing approach identical to our local one and I've verified that this PR's commit runs fine through Travis when using that approach.

I will reopen this PR once it incorporates the merged version of PR #28