Re: replicating DROP commands across servers - Mailing list pgsql-hackers

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:

>           case OCLASS_PROC:
>               appendStringInfoString(&buffer,
>                                  format_procedure_qualified(object->objectId));
> +             if (objname)
> +                 format_procedure_parts(object->objectId, objname, objargs);
>               break;
>
>           case OCLASS_TYPE:
> !             {
> !                 char *typeout;
> !
> !                 typeout = format_type_be_qualified(object->objectId);
> !                 appendStringInfoString(&buffer, typeout);
> !                 if (objname)
> !                     *objname = list_make1(typeout);
> !             }
>               break;

here's a "throw away whatever was in objname" case.

> ***************
> *** 2745,2750 **** getObjectIdentity(const ObjectAddress *object)
> --- 2991,3000 ----
>                                 format_type_be_qualified(castForm->castsource),
>                                format_type_be_qualified(castForm->casttarget));
>
> +                 if (objname)
> +                     *objname = list_make2(format_type_be_qualified(castForm->castsource),
> +                                           format_type_be_qualified(castForm->casttarget));
> +
>                   heap_close(castRel, AccessShareLock);
>                   break;
>               }

And another "throw away" case.

> --- 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.

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.

I'm also not a huge fan of the big object_type_map, but I also don't
have a better solution.
Thanks,
    Stephen