Re: Column Filtering in Logical Replication

On 2021-Dec-10, Peter Eisentraut wrote:

> I looked through this a bit.  You had said that you are still going to
> integrate past review comments, so I didn't look to deeply before you get to
> that.

Thanks for doing this!  As it happens I've spent the last couple of days
working on some of these details.

> There was no documentation, so I wrote a bit (patch 0001).  It only touches
> the CREATE PUBLICATION and ALTER PUBLICATION pages at the moment.  There was
> no mention in the Logical Replication chapter that warranted updating.
> Perhaps we should revisit that chapter at the end of the release cycle.

Thanks.  I hadn't looked at the docs yet, so I'll definitely take this.

> DDL tests should be done in src/test/regress/sql/publication.sql rather than
> through TAP tests, to keep it simpler.

Yeah, I noticed this too but hadn't done it yet.

> Note the FIXME marker that it does not recognize if the
> listed columns don't exist.

I had fixed this already, so I suppose it should be okay.

> I removed a now redundant test from the TAP
> test file.  The other error condition test in the TAP test file
> ('publication relation test_part removed') I didn't understand: test_part
> was added with columns (a, b), so why would dropping column b remove the
> whole entry?  Maybe I missed something, or this could be explained better.

There was some discussion about it earlier in the thread and I was also
against this proposed behavior.

> I was curious what happens when you have different publications with
> different column lists, so I wrote a test for that (patch 0003).  It turns
> out it works, so there is nothing to do, but perhaps the test is useful to
> keep.

Great, thanks.  Yes, I think it will be.

> On the implementation side, I think the added catalog column
> pg_publication_rel.prattrs should be an int2 array, not a text array.

I already rewrote it to use a int2vector column in pg_publication_rel.
This interacted badly with the previous behavior on dropping columns,
which I have to revisit, but otherwise it seems much better.
(Particularly since we don't need to care about quoting names and such.)

> Finally, I suggest not naming this feature "column filter".  I think this
> name arose because of the analogy with the "row filter" feature also being
> developed.  But a filter is normally a dynamic data-driven action, which
> this is not.  Golden Gate calls it in their documentation "Selecting
> Columns", or we could just call it "column list".

Hmm, I hadn't thought of renaming the feature, but I have to admit that
I was confused because of the name, so I agree with choosing some other
name.

I'll integrate your changes and post the whole thing later.

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/
Si no sabes adonde vas, es muy probable que acabes en otra parte.