Re: Column Filtering in Logical Replication

On 2021-Sep-06, Rahila Syed wrote:

> > > ... ugh.  Since CASCADE is already defined to be a
> > > potentially-data-loss operation, then that may be acceptable
> > > behavior.  For sure the default RESTRICT behavior shouldn't do it,
> > > though.
> >
> > That makes sense to me.
>
> However, the default (RESTRICT) behaviour of DROP TABLE allows
> removing the table from the publication. I have implemented the
> removal of table from publication on drop column (RESTRICT)  on the
> same lines.

But dropping the table is quite a different action from dropping a
column, isn't it?  If you drop a table, it seems perfectly reasonable
that it has to be removed from the publication -- essentially, when the
user drops a table, she is saying "I don't care about this table
anymore".  However, if you drop just one column, that doesn't
necessarily mean that the user wants to stop publishing the whole table.
Removing the table from the publication in ALTER TABLE DROP COLUMN seems
like an overreaction.  (Except perhaps in the special case were the
column being dropped is the only one that was being published.)

So let's discuss what should happen.  If you drop a column, and the
column is filtered out, then it seems to me that the publication should
continue to have the table, and it should continue to filter out the
other columns that were being filtered out, regardless of CASCADE/RESTRICT.
However, if the column is *included* in the publication, and you drop
it, ISTM there are two cases:

1. If it's DROP CASCADE, then the list of columns to replicate should
continue to have all columns it previously had, so just remove the
column that is being dropped.

2. If it's DROP RESTRICT, then an error should be raised so that the
user can make a concious decision to remove the column from the filter
before dropping the column.

> Did you give any thoughts to my earlier suggestion related to syntax [1]?
> 
> [1] https://www.postgresql.org/message-id/CAA4eK1J9b_0_PMnJ2jq9E55bcbmTKdUmy6jPnkf1Zwy2jxah_g%40mail.gmail.com

This is a great followup idea, after the current feature is committed.
There are a few things that have been reported in review comments; let's
get those addressed before adding more features on top.

I pushed the clerical part of this -- namely the addition of
PublicationTable node and PublicationRelInfo struct.  I attach the part
of your v4 patch that I didn't include.  It contains a couple of small
corrections, but I didn't do anything invasive (such as pgindent)
because that would perhaps cause you too much merge pain.

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/