Hi, Thank you for updating the patch. The regression tests and tap tests pass with v9 patch.
> > After working on this a little bit more, I realized that this is a bad > idea overall. It causes lots of complications and it's just not worth > it. So I'm back at my original thought that we need to throw an ERROR > at ALTER TABLE .. DROP COLUMN time if the column is part of a > replication column filter, and suggest the user to remove the column > from the filter first and reattempt the DROP COLUMN. > > This means that we need to support changing the column list of a table > in a publication. I'm looking at implementing some form of ALTER > PUBLICATION for that. > > I think right now the patch contains support only for ALTER PUBLICATION.. ADD TABLE with column filters. In order to achieve changing the column lists of a published table, I think we can extend the ALTER TABLE ..SET TABLE syntax to support specification of column list. So this whole thing can > be reduced to just this: if (att_map != NULL && !bms_is_member(att->attnum, att_map)) > continue; /* that is, don't send this attribute */ I agree the condition can be shortened now. The long if condition was included because initially the feature allowed specifying filters without replica identity columns(sent those columns internally without user having to specify). 900 + * the table is partitioned. Run a recursive query to iterate > through all > 901 + * the parents of the partition and retreive the record for > the parent > 902 + * that exists in pg_publication_rel. > 903 + */ The above comment in fetch_remote_table_info() can be changed as the recursive query is no longer used. Thank you, Rahila Syed
