Re: replicating DROP commands across servers - Mailing list pgsql-hackers
From | Alvaro Herrera |
---|---|
Subject | Re: replicating DROP commands across servers |
Date | |
Msg-id | 20141003202310.GH7043@eldon.alvh.no-ip.org Whole thread Raw |
In response to | Re: replicating DROP commands across servers (Stephen Frost <sfrost@snowman.net>) |
Responses |
Re: replicating DROP commands across servers
|
List | pgsql-hackers |
Stephen Frost wrote: > Alvaro, > > * Alvaro Herrera (alvherre@2ndquadrant.com) wrote: > > + /* > > + * Make sure that both objname and objargs were passed, or none was. > > + * Initialize objargs to empty list, which is the most common case. > > + */ > > + Assert(PointerIsValid(objname) == PointerIsValid(objargs)); > > + if (objargs) > > + *objargs = NIL; > > + > > I feel like I must be missing something, but you only explicitly reset > objargs, not objnames, and then below you sometimes add to objname and > other times throw away anything which might be there: > > > --- 2948,2974 ---- > > attr = get_relid_attribute_name(object->objectId, > > object->objectSubId); > > appendStringInfo(&buffer, ".%s", quote_identifier(attr)); > > + if (objname) > > + *objname = lappend(*objname, attr); > > } > > break; > > Here's an "add to objname" case, and then: Right. In the "add to objname" cases, there is already some other routine that initialized it previously by filling in some stuff; in the case above, this happens in the getRelationIdentity() immediately preceding this. In the other cases we initialize on that spot. > > --- 3037,3045 ---- > > { > > appendStringInfo(&buffer, "%s on ", > > quote_identifier(NameStr(con->conname))); > > ! getRelationIdentity(&buffer, con->conrelid, objname); > > ! if (objname) > > ! *objname = lappend(*objname, pstrdup(NameStr(con->conname))); > > } > > else > > { > > And another "add to existing" case. Note how this one has a getRelationIdentity, just like the first one. > Guess I have a bit of a hard time with an API that's "we might add to > this list, or we might replace whatever is there". I think it would be > best to just initialize both (or assert that they are) and have any > callers who need to merge the list(s) coming back into an existing list > handle that themselves. The thing is, the list is already initialized in all cases to a valid list in this routine; there is no case that appends to whatever junk might have been there before this routine started. > I'm also not a huge fan of the big object_type_map, but I also don't > have a better solution. Agreed. We have the ObjectProperty array also in the same file; it kinda looks like there is duplication here, but actually there isn't. This whole issue is just fallout from the fact that we have three different ways to identify object classes: the ObjectClass enum, the ObjectType enum, and the relation OIDs of each catalog (actually a fourth one, see below). I don't see any other nice way around this; I guess we could try to auto-generate these tables somehow from a master text file, or something. Not material for this patch, I think. Note my DDL deparse patch adds a comment: +/* XXX merge this with ObjectTypeMap? */static event_trigger_support_data event_trigger_support[] = { and a late revision to that patch added a new function in event_triggers.c (not yet posted I think) due to GRANT having its own enum of object types, AclObjectKind. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
pgsql-hackers by date: