| From: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
|---|---|
| To: | Petr Jelinek <petr(at)2ndquadrant(dot)com> |
| Cc: | Steve Singer <steve(at)ssinger(dot)info>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Logical Replication WIP |
| Date: | 2016-11-08 18:51:28 |
| Message-ID: | a8b7afab-1970-e198-7c0a-2603c066ee14@2ndquadrant.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Review of v7 0003-Add-PUBLICATION-catalogs-and-DDL.patch:
This appears to address previous reviews and is looking pretty solid. I
have some comments that are easily addressed:
[still from previous review] The code for OCLASS_PUBLICATION_REL in
getObjectIdentityParts() does not fill in objname and objargs, as it is
supposed to.
catalog.sgml: pg_publication_rel column names must be updated after renaming
alter_publication.sgml and elsewhere: typos PuBLISH_INSERT etc.
create_publication.sgml: FOR TABLE ALL IN SCHEMA does not exist anymore
create_publication.sgml: talks about not-yet-existing SUBSCRIPTION role
DropPublicationById maybe name RemovePublicationById for consistency
system_views.sql: C.relkind = 'r' unnecessary
CheckCmdReplicaIdentity: error message says "cannot update", should
distinguish between update and delete
relcache.c: pubactions->pubinsert |= pubform->pubinsert; etc. should be ||=
RelationData.rd_pubactions could be a bitmap, simplifying some memcpy
and context management. But RelationData appears to favor rich data
structures, so maybe that is fine.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Eisentraut | 2016-11-08 19:04:47 | Re: Logical Replication WIP |
| Previous Message | Clifford Hammerschmidt | 2016-11-08 18:48:40 | Re: C based plugins, clocks, locks, and configuration variables |