| From: | vignesh C <vignesh21(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Distinguish publication exclusions in object addresses |
| Date: | 2026-09-16 07:37:36 |
| Message-ID: | CALDaNm2L3pxVD30YN_tbJ8SSCCJpxZam7zaU4+N+kVyY1XZwug@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 16 Sept 2026 at 12:16, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Sep 16, 2026, at 13:50, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > --
> > With Regards,
> > Amit Kapila.
> > <v4-0001-Distinguish-publication-exclusions-in-object-addr.patch>
>
> V4 overall looks sold to me. Just a couple of small comments:
>
> 1
> ```
> -- No entry of either kind. testpub_default publishes nothing.
> SELECT pg_get_object_address('publication excluded relation',
> '{public, testpub_tbl1}', '{testpub_default}');
> ERROR: publication relation "testpub_tbl1" in publication "testpub_default" does not exist
> ```
>
> For this new test, the error message is a little surprising to me. Since the requested object type is "publication excluded relation", I would expect the error message to say something like:
> ```
> publication excluded relation "testpub_tbl1" in publication "testpub_default" does not exist
> ```
I think it would add some unnecessary code complexity in this case.
Since the underlying issue is simply that the relation is not present
in the publication, I think the existing generic error message is
sufficient and should be understandable in the context of publication
excluded relation.
> 2
> ```
> + if (objtype == OBJECT_PUBLICATION_EXCLUDED_REL && !isexcept)
> + ereport(ERROR,
> + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("\"%s\" is not an excluded relation of publication \"%s\"",
> + RelationGetRelationName(relation), pubname)));
> + else if (objtype == OBJECT_PUBLICATION_REL && isexcept)
> + ereport(ERROR,
> + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("\"%s\" is not a published relation of publication \"%s\"",
> + RelationGetRelationName(relation), pubname)));
> ```
>
> Nitpick: with the modern ereport() style, the extra parentheses around errcode() and errmsg() are no longer needed.
This is consistent with the nearby code, which already uses the same
style with the extra parentheses. Since this is not a new function, I
think it is better to keep it consistent with the surrounding code.
Regards,
Vignesh
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2026-09-16 07:41:15 | Re: Distinguish publication exclusions in object addresses |
| Previous Message | Amit Kapila | 2026-09-16 07:34:03 | Re: Distinguish publication exclusions in object addresses |