I have addressed the comments. Attached the updated patch.
[1]: https://www.postgresql.org/message-id/CAA4eK1Jjm+w3hEGgsDu_r1Pysez=8mmtGu6=XwPE4MuH+eYG8Q@mail.gmail.com
A few minor comments:
1. Add a negative test for missing table keyword for example EXCEPT (t1, t2)
2. NIT comment, remove the extra space between parenthesis and TABLE below
else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES"))
- COMPLETE_WITH("EXCEPT TABLE (", "WITH (");
+ COMPLETE_WITH("EXCEPT ( TABLE", "WITH (");
3. for pg_dump, to improve readability can you just add TABLE keyword for every tables in the except list?
- /* Include EXCEPT TABLE clause if there are except_tables. */
+ /* Include EXCEPT (TABLE) clause if there are except_tables. */
for (SimplePtrListCell *cell = pubinfo->except_tables.head; cell; cell = cell->next)
{
TableInfo *tbinfo = (TableInfo *) cell->ptr;
if (++n_except == 1)
- appendPQExpBufferStr(query, " EXCEPT TABLE (");
+ appendPQExpBufferStr(query, " EXCEPT (TABLE ");
else
appendPQExpBufferStr(query, ", ");
4. Is the Assert needed below, code already produces error message.
+ Assert(pubobj->pubobjtype == PUBLICATIONOBJ_EXCEPT_TABLE);
+
+ if (pubobj->pubobjtype == PUBLICATIONOBJ_CONTINUATION)
+ ereport(ERROR,
+ errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("invalid publication object list"),
+ errdetail("TABLE must be specified before a standalone table."),
+ parser_errposition(pubobj->location));
Thanks,
Satya
