On Monday, September 26, 2022 4:57 PM Amit Kapila <amit.kapil...@gmail.com> > > On Mon, Sep 26, 2022 at 2:03 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> > wrote: > > > > On 2022-Sep-26, Amit Kapila wrote: > > > > > On Mon, Sep 26, 2022 at 1:10 PM Alvaro Herrera > <alvhe...@alvh.no-ip.org> wrote: > > > > > > ERROR: cannot use column list for relation "%s.%s" in publication "%s" > > > > DETAIL: Column lists cannot be specified in publications containing FOR > TABLES IN SCHEMA elements. > > > > > > This looks mostly good to me. BTW, is it a good idea to add ".. in > > > publication "%s"" to the error message as this can happen even > > > during create publication? > > > > Hmm, I don't see why not. The publication is new, sure, but it would > > already have a name, so there's no possible confusion as to what this > > means. > > > > (My main change was to take the word "publication" out of the phrase > > "publication column list", because that seemed a bit strange; it isn't > > the publication that has a column list, but the relation.) > > > > Okay, that makes sense.
+1 > > > > > If so, I think we can change the nearby message as below to include > > > the same: > > > > > > if (!pubviaroot && > > > pri->relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) > > > ereport(ERROR, > > > (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > > > errmsg("cannot use publication column list for relation \"%s\"", > > > > WFM. > > > > Okay. While reviewing this part, I notice an unused parameter(queryString) of function CheckPubRelationColumnList. I feel we can remove that as well while on it. I plan to post a patch to fix the error message and parameter soon. Best regards, Hou zj