Dear Shubham,

More comments for v43-0001.

01. publication.out and publication.sql

I think your fix is not sufficient, even if it pass tests. 

```
-- error: system attributes "ctid" not allowed in column list
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, ctid);
-ERROR:  cannot use system column "ctid" in publication column list
+ERROR:  relation "testpub_tbl5" is already member of publication 
"testpub_fortable"
 ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl1 (id, ctid);
 ERROR:  cannot use system column "ctid" in publication column list
 -- error: duplicates not allowed in column list
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, a);
-ERROR:  duplicate column "a" in publication column list
+ERROR:  relation "testpub_tbl5" is already member of publication 
"testpub_fortable
```

The error message is not match with the comment. The comment said that the table
has already been added in the publication. I think the first line [1] succeeded 
by your change
and testpub_tbl5 became a member at that time then upcoming ALTER statements 
failed
by the duplicate registration.

```
-- ok
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c);
+ERROR:  relation "testpub_tbl5" is already member of publication 
"testpub_fortable"
```

You said OK but same error happened.

```
ALTER TABLE testpub_tbl5 DROP COLUMN c;     -- no dice
-ERROR:  cannot drop column c of table testpub_tbl5 because other objects 
depend on it
-DETAIL:  publication of table testpub_tbl5 in publication testpub_fortable 
depends on column c of table testpub_tbl5
-HINT:  Use DROP ... CASCADE to drop the dependent objects too.
```

This statement should be failed because c was included in the column.
However, it succeeded because previous ALTER PUBLICATION was failed.
Upcoming SQLs wrongly thawed ERRORs because of this.

Please look at all of differences before doing copy-and-paste.

02. 031_column_list.pl

```
-# TEST: Generated and dropped columns are not considered for the column list.
+# TEST: Dropped columns are not considered for the column list.
 # So, the publication having a column list except for those columns and a
 # publication without any column (aka all columns as part of the columns
 # list) are considered to have the same column list.
```

Based on the comment, this case does not test the behavior of generated columns
anymore. So, I felt column 'd' could be removed from the case.

03. 031_column_list.pl

Can we test that generated columns won't be replaced if it does not included in
the column list?

[1]:
```
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, d);
```

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Reply via email to