This sounds like a good approach.

> +static void
> +pg_decode_truncate(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
> +                                int nrelations, Relation relations[], 
> ReorderBufferChange *change)
> +{

> +     for (i = 0; i < nrelations; i++)
> +     {
> +             Oid                     relid = RelationGetRelid(relations[i]);
> +
> +             if (i > 0)
> +                     appendStringInfoString(ctx->out, ", ");
> +
> +             appendStringInfoString(ctx->out,
> +                                                        
> quote_qualified_identifier(
> +                                                                
> get_namespace_name(get_rel_namespace(relid)),
> +                                                                
> get_rel_name(relid)));

Note that you start the loop having the Relation; yet you go extra
length to grab the relnamespace and relname from syscache instead of
just relations[i]->rd_rel->relname etc.

pgoutput doesn't do it that way, so it doesn't affect logical
replication, but I think it's best not to create awkward code in
test_decoding, since it's so widely copied.


> +     else if (info == XLOG_HEAP_TRUNCATE)
> +     {
> +             xl_heap_truncate *xlrec = (xl_heap_truncate *) rec;
> +
> +             if (xlrec->flags & XLH_TRUNCATE_CASCADE)
> +                     appendStringInfo(buf, "cascade ");
> +             if (xlrec->flags & XLH_TRUNCATE_RESTART_SEQS)
> +                     appendStringInfo(buf, "restart_seqs ");
> +             appendStringInfo(buf, "nrelids %u", xlrec->nrelids);
> +             /* skip the list of relids */
> +     }

Maybe not a big deal, but for future pg_waldump users I'm sure it'll be
nice to have the OIDs here.

> +void
> +ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
> +                                                     DropBehavior behavior, 
> bool restart_seqs)
> +{

Please add a comment atop this function.

  
> +     /*
> +      * Write a WAL record to allow this set of actions to be logically 
> decoded.
> +      *
> +      * Assemble an array of relids so we can write a single WAL record for 
> the
> +      * whole action.
> +      */
> +     if (list_length(relids_logged) > 0)
> +     {
> +             xl_heap_truncate xlrec;
> +             int                     i = 0;

I wonder if this should happen only if logical decoding?  (Maybe it
already occurs because relids_logged would be empty?  Worth a comment in
that case)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply via email to