On Tue, May 17, 2022 at 1:56 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, May 17, 2022 at 7:35 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > Below are my review comments for v5-0002. > > > > There may be an overlap with comments recently posted by Osumi-san [1]. > > > > (I also have review comments for v5-0002; will post them tomorrow) > > > > ====== > > > > 1. General > > > > Is it really necessary to have to say "EXCEPT TABLE" instead of just > > "EXCEPT". It seems unnecessarily verbose and redundant when you write > > "FOR ALL TABLES EXCEPT TABLE...". > > > > If you want to keep this TABLE keyword (maybe you have plans for other > > kinds of except?) > > > > I don't think there is an immediate plan but one can imagine using > EXCEPT SCHEMA. Then for column lists, one may want to use the syntax > Create Publication pub1 For Table t1 Except Cols (c1, ..); > > > then IMO perhaps at least it can be the optional > > default except type. e.g. EXCEPT [TABLE]. > > > > Yeah, that might be okay, so, even if we plan to extend this in the > future, by default we will consider the list of tables after EXCEPT > but if the user mentions EXCEPT SCHEMA or something else then we can > use a different object. Is that sound okay?
Yes. That is what I meant. > > > > > 3. General > > > > The ONLY keyword seems supported by the syntax for tables of the > > except-list (more on this in later comments) but: > > a) I am not sure if the patch code is accounting for that, and > > b) There are no test cases using ONLY. > > > > ~~~ > > > > Isn't it better to map ONLY with the way it can already be specified > in CREATE PUBLICATION? I am not sure what exactly is proposed and what > is your suggestion? Can you please explain if it is different from the > way we use it for CREATE PUBLICATION? > Yes, I am not proposing anything different to how ONLY already works for published tables. I was only questioning whether the patch behaves correctly when ONLY is specified for the tables of the EXCEPT list. I had some doubt about it because there are a few other review comments I wrote (e.g. in pg_dump.c), and also I did not find any ONLY tests, ------ Kind Regards, Peter Smith. Fujitsu Australia