Re: pg_upgrade 9.4 -> 9.5 with pg_trgm fails for me

On Fri, Jan 08, 2016 at 12:53:24PM -0500, Tom Lane wrote:

> >> and here is the function that leads to the schema having a
> >> dependancy on table data:
>
> Hm. So, by having installed this function as a check constraint, you have
> created a data dependency that pg_dump has no way to know about. It's
> going to load the tables in some order that's chosen without regard to the
> need for dem.staff to be populated first. This is not a pg_dump bug.

Yes, I agree.

> In general, embedding lookups of other tables into CHECK constraints
> is going to cause you all kinds of grief quite aside from pg_dump
> not understanding it, because the backend doesn't really understand it
> either. If the other table changes, causing the CHECK expression to
> fail, that will *not* cause anything to happen to the table with the
> CHECK constraint. It could well be that pg_dump is loading the tables
> in the right order by chance, and the reason you're seeing a failure
> is that one or more rows have modified_by values corresponding to
> people who no longer are in the staff table.

Not really but for that I need to deliver more information.
The audit.audit_fields table is part of GNUmed's homegrown,
trigger based audit solution:

- tables have audit tables w/o constraints in the audit. schema
- triggers on tables log UPDATEs/DELETEs into the audit tables
- tables being audited (such as dem.staff) INHERIT from audit.audit_fields
- audit.audit_fields is never inserted into directly (only into child tables)
- audit.audit_fields carries the constraint based on gm.is_dbowner_or_staff()

(so, yes, it is even worse: since dem.staff is audited, and
therefore inherits the check constraint, it depends on itself :-o

Table "dem.staff"
Column | Type | Modifiers | Storage | Stats target | Description
---------------+--------------------------+-----------------------------------------------------------------------+----------+--------------+-----------------------------------------------------------
pk_audit | integer | not null default nextval('audit.audit_fields_pk_audit_seq'::regclass) | plain | |
row_version | integer | not null default 0 | plain | |
modified_when | timestamp with time zone | not null default now() | plain | |
modified_by | name | not null default "current_user"() | plain | |
pk | integer | not null default nextval('dem.staff_pk_seq'::regclass) | plain | |
fk_identity | integer | not null | plain | |
db_user | name | not null default "current_user"() | plain | |
short_alias | text | not null | extended | | a short signature unique to this staff member +
| | | | | to be used in the GUI, actually this is somewhat+
| | | | | redundant with ext_person_id...
comment | text | | extended | |
is_active | boolean | not null default true | plain | |
Indexes:
"staff_pkey" PRIMARY KEY, btree (pk)
"staff_db_user_key" UNIQUE CONSTRAINT, btree (db_user)
"staff_short_alias_key" UNIQUE CONSTRAINT, btree (short_alias)
Foreign-key constraints:
"staff_fk_identity_fkey" FOREIGN KEY (fk_identity) REFERENCES dem.identity(pk) ON UPDATE CASCADE ON DELETE CASCADE
Referenced by:
TABLE "bill.bill_item" CONSTRAINT "bill_item_fk_provider_fkey" FOREIGN KEY (fk_provider) REFERENCES dem.staff(pk) ON UPDATE CASCADE ON DELETE RESTRICT
TABLE "blobs.doc_obj" CONSTRAINT "doc_obj_fk_intended_reviewer_fkey" FOREIGN KEY (fk_intended_reviewer) REFERENCES dem.staff(pk) ON UPDATE CASCADE ON DELETE RESTRICT
TABLE "dem.identity" CONSTRAINT "identity_fk_primary_provider_fkey" FOREIGN KEY (fk_primary_provider) REFERENCES dem.staff(pk) ON UPDATE CASCADE ON DELETE RESTRICT
TABLE "clin.incoming_data_unmatched" CONSTRAINT "incoming_data_unmatched_fk_provider_disambiguated_fkey" FOREIGN KEY (fk_provider_disambiguated) REFERENCES dem.staff(pk) ON UPDATE CASCADE ON DELETE RESTRICT
TABLE "ref.keyword_expansion" CONSTRAINT "keyword_expansion_fk_staff_fkey" FOREIGN KEY (fk_staff) REFERENCES dem.staff(pk)
TABLE "dem.message_inbox" CONSTRAINT "provider_inbox_fk_staff_fkey" FOREIGN KEY (fk_staff) REFERENCES dem.staff(pk)
TABLE "clin.review_root" CONSTRAINT "review_root_fk_reviewer_fkey" FOREIGN KEY (fk_reviewer) REFERENCES dem.staff(pk) ON UPDATE CASCADE ON DELETE RESTRICT
TABLE "clin.reviewed_test_results" CONSTRAINT "reviewed_test_results_fk_reviewer_fkey" FOREIGN KEY (fk_reviewer) REFERENCES dem.staff(pk) ON UPDATE CASCADE ON DELETE RESTRICT
TABLE "clin.test_result" CONSTRAINT "test_result_fk_intended_reviewer_fkey" FOREIGN KEY (fk_intended_reviewer) REFERENCES dem.staff(pk) ON UPDATE CASCADE ON DELETE RESTRICT
TABLE "clin.vaccination" CONSTRAINT "vaccination_fk_provider_fkey" FOREIGN KEY (fk_provider) REFERENCES dem.staff(pk) ON UPDATE CASCADE ON DELETE RESTRICT
Triggers:
zt_del_staff BEFORE DELETE ON dem.staff FOR EACH ROW EXECUTE PROCEDURE audit.ft_del_staff()
zt_ins_staff BEFORE INSERT ON dem.staff FOR EACH ROW EXECUTE PROCEDURE audit.ft_ins_staff()
zt_upd_staff BEFORE UPDATE ON dem.staff FOR EACH ROW EXECUTE PROCEDURE audit.ft_upd_staff()
Inherits: audit.audit_fields

Table "audit.audit_fields"
Column | Type | Modifiers | Storage | Stats target | Description
---------------+--------------------------+-----------------------------------------------------------------------+---------+--------------+--------------------------------------------------------
pk_audit | integer | not null default nextval('audit.audit_fields_pk_audit_seq'::regclass) | plain | |
row_version | integer | not null default 0 | plain | | the version of the row; mainly just a count
modified_when | timestamp with time zone | not null default now() | plain | | when has this row been committed (created/modified)
modified_by | name | not null default "current_user"() | plain | | by whom has this row been committed (created/modified)
Indexes:
"audit_fields_pkey" PRIMARY KEY, btree (pk_audit)
Rules:
audit_fields_no_del AS
ON DELETE TO audit.audit_fields DO INSTEAD NOTHING
audit_fields_no_ins AS
ON INSERT TO audit.audit_fields DO INSTEAD NOTHING
audit_fields_no_upd AS
ON UPDATE TO audit.audit_fields DO INSTEAD NOTHING
Child tables: bill.bill,
bill.bill_item,
blobs.doc_desc,
blobs.doc_med,
blobs.lnk_doc2hospital_stay,
blobs.lnk_doc_med2episode,
cfg.report_query,
clin.allergy_state,
clin.clin_diag,
clin.clin_item_type,
clin.clin_root_item,
clin.encounter,
clin.episode,
clin.external_care,
clin.fhx_relation_type,
clin.form_data,
clin.health_issue,
clin.incoming_data_unmatchable,
clin.incoming_data_unmatched,
clin.lnk_code2item_root,
clin.lnk_constraint2vacc_course,
clin.lnk_pat2vaccination_course,
clin.lnk_substance2episode,
clin.lnk_tst2norm,
clin.lnk_type2item,
clin.lnk_vaccination_course2schedule,
clin.lnk_vaccine2inds,
clin.patient,
clin.review_root,
clin.suppressed_hint,
clin.test_org,
clin.test_panel,
clin.test_type,
clin.vaccination_course,
clin.vaccination_course_constraint,
clin.vaccination_definition,
clin.vaccination_schedule,
clin.vacc_indication,
clin.vaccine,
clin.vaccine_batches,
clin.vacc_route,
clin.waiting_list,
de_de.beh_fall_typ,
de_de.lab_test_gnr,
de_de.prax_geb_paid,
dem.address,
dem.gender_label,
dem.identity,
dem.identity_tag,
dem.inbox_item_category,
dem.inbox_item_type,
dem.lnk_identity2ext_id,
dem.lnk_job2person,
dem.lnk_org_unit2comm,
dem.lnk_org_unit2ext_id,
dem.lnk_person2relative,
dem.message_inbox,
dem.occupation,
dem.org,
dem.org_unit,
dem.praxis_branch,
dem.relation_types,
dem.staff,
dem.state,
dem.street,
dem.urb,
gm.access_log,
ref.auto_hint,
ref.branded_drug,
ref.consumable_substance,
ref.data_source,
ref.lnk_substance2brand,
ref.paperwork_templates,
ref.tag_image

> Can you get rid of dem.staff in favor of something like creating a
> "staff" role and GRANT'ing that to appropriate users?

We already use PG roles per app user and group roles to
aggregate roles into permission groups. App user are defined
in ... dem.staff.

> Alternatively, maybe you can make the modified_by column be a foreign
> key

The gm.is_staff_or_dbowner() business is all about a
desperate attempt to overcome the inability to define foreign
keys into system tables.

Would it be an idea to be able to define such foreign keys
but not enforce them / disable them such that pg_dump could
know about them ? More like hints ? I realize this wouldn't
magically make pg_dump know about my hack...

> referencing a table of users (it probably couldn't be defined
> quite like "staff", but you get the idea). The presence of the foreign
> key would be enough to cue pg_dump about load order.

Hm, hm, OK, that might be an idea. It would be a superset of
dem.staff (namely also including "postgres" and the db-owner).

I will pursue this idea.

Karsten
--
GPG key ID E4071346 @ eu.pool.sks-keyservers.net
E167 67FD A291 2BEA 73BD 4537 78B9 A9F9 E407 1346