On Mon, Aug 30, 2021 at 8:56 PM vignesh C <vignes...@gmail.com> wrote: > > On Mon, Aug 30, 2021 at 9:10 AM houzj.f...@fujitsu.com > <houzj.f...@fujitsu.com> wrote: > > > > > 5) > > + if (list_length(pubobj->name) == 1 && > > + (strcmp(relname, "CURRENT_SCHEMA") == 0)) > > + ereport(ERROR, > > + > > errcode(ERRCODE_SYNTAX_ERROR), > > + errmsg("invalid relation > > name at or near"), > > + parser_errposition(pstate, > > pubobj->location)); > > > > Maybe we don't need this check, because it will report an error in > > OpenTableList() anyway, "relation "CURRENT_SCHEMA" does not exist" , and > > that > > message seems readable to me. > > Allowing CURRENT_SCHEMA is required to support current schema for > schema publications, currently I'm allowing this syntax during parsing > and this error is thrown for relations later, this is done to keep the > similar error as earlier before this feature support. I felt we can > keep it like this to maintain the similar error. Thoughts? >
I find this check quite ad-hoc in the code and I am not sure if we need to be consistent for the exact message in this case. So, I think it is better to remove it. > > > About 0003 > > 7) > > The v22-0003 seems simple and can remove lots of code in patch v22-0001, so > > maybe we can merge 0001 and 0003 into one patch ? > > I agree that the code becomes simpler, it reduces a lot of code. I had > kept it like that as the testing effort might be more and also I was > waiting if there was no objection for that syntax from anyone else. I > will wait for a few more reviews and merge it to 0001 if there are no > objections. > +1 to merge the patch as suggested by Hou-San. -- With Regards, Amit Kapila.