On Monday, September 27, 2021 12:32 PM vignesh C <vignesh21@gmail.com> wrote:
> On Fri, Sep 24, 2021 at 11:46 AM vignesh C <vignesh21@gmail.com> wrote:
> >
>
> Attached v33 patch has the preprocess_pubobj_list review comment fix
> suggested by Alvaro at [1]. The
> v33-0006-Alternate-grammar-for-ALL-TABLES-IN-SCHEMA.patch patch has
> the grammar changes as suggested by Alvaro at [1]. If we agree this is better, I
> will merge this into the 0001 patch.
> [1] - https://www.postgresql.org/message-id/202109241325.eag5g6mpvoup%40alvherre.pgsql
Hi,
The grammar change basically looks good to me. Only one suggestion is that it
will be better to add some more comments in gram.y to describe the rule
PublicationObjSpec. Because it's a new style syntax in postgresql, people might
wonder how the code work and why we choose this design when they first time
see this rule in gram.y.
Maybe something like the following:
+/*
+ * FOR TABLE and FOR ALL TABLES IN SCHEMA specifications
+ *
+ * This rule parses publication object with and without keyword prefix.
+ *
+ * The actual type of the object without keyword prefix depends on the previous
+ * one with keyword prefix. It will be preprocessed in preprocess_pubobj_list().
+ *
+ * For the object without keyword prefix, we cannot just use relation_expr here,
+ * because some extended expression in relation_expr cannot be used as a
+ * schemaname and we cannot differentiate it. So, we extract the rules from
+ * relation_expr here.
+ */
PublicationObjSpec:
TABLE relation_expr
...
My words might not be good, but I think it will be better to add some comments
to explain a bit about the code in gram.y. Thoughts ?
Best regards,
Hou zj