| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
| Cc: | Takeshi Ideriha <iderihatakeshi(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Publication of same parent table with ONLY-ness mismatch should error |
| Date: | 2026-10-19 07:10:52 |
| Message-ID: | CAJpy0uC4TuqpTMv90P9p5ssCwXq5SReot7=DBSvcLoFKwVnVbg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Oct 19, 2026 at 11:50 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Thu, Oct 15, 2026 at 5:51 PM Takeshi Ideriha
> <iderihatakeshi(at)gmail(dot)com> wrote:
> >
> > Hi all
> >
> > >On 2026/08/28 13:54, shveta malik wrote:
> > >> On Fri, Aug 28, 2026 at 9:04 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > >>
> > >> Hi.
> > >>
> > >> While reviewing another thread, I had cause to enumerate and try many
> > >> dozens of CREATE PUBLICATION clause combinations. I found one
> > >> combination that gave an unexpected result. IMO this a bug:
> > >>
> > >> CREATE PUBLICATION ... FOR TABLE s1.parent, TABLE ONLY s1.parent;
> > >> -- No error? First wins. Behaves like FOR TABLE
> > >>
> > >> CREATE PUBLICATION ... FOR TABLE ONLY s1.parent, TABLE s1.parent;
> > >> -- No error? First wins. Behaves like FOR TABLE ONLY
> >
> > I also think this is a bug and should become an error. Specifying both
> > TABLE and TABLE ONLY does not make much sense.
> >
> >
> > > I also feel that it should be an error scenario. Let's see what others
> > > have to say here.
> > >
> > > While validating the patch, I figured out that this simple scenario,
> > > even without inheritance created, also triggers the error:
> > >
> > > postgres=# create table tab1(i int);
> > > CREATE TABLE
> > > postgres=# create publication pub1 for table only tab1, tab1;
> > > ERROR: conflicting ONLY specifications for table "tab1"
> > >
> > > I see that 't->relation->inh' is true for anything which does not have
> > > 'ONLY' specified with it. It makes sense, too. Even though tab1
> > > doesn't have any descendants yet, it may have them in the future, and
> > > then this publication can break silently. So, it's better to reject
> > > such a publication at creation time, even for tables like tab1.
> >
> > For a plain table with no child tables, I also feel it make sense to
> > make this error. It seems to me a publication table is registered in
> > pg_publication_rel based on the definition when CREATE PUBLICATION is
> > executed. So, even if the plain table gets a child table in future, it
> > would not affect the publication definition. Having said that, this
> > should become an error.
> >
> > The fix and test code look good to me. I also noticed that you improved
> > the doc explanation regarding the relationship between ONLY clause and
> > partitioned table. How about splitting this doc patch to a separate
> > patch and thread? The doc fix seems good to me but it's not directly
> > related to the main issue in this thread. By separating the thread, I
> > guess it would get more attention and be easier to commit.
>
> Hi Takeshi-San.
>
> Thanks for giving feedback on this patch.
>
> Bugfix:
> I found a scenario that was not being handled correctly -- i.e. when
> implicitly inherited children were also explicitly mentioned.
> Fixed and added a test case.
>
> Docs patch:
> The docs part is an independent improvement, so I'll move it to a
> separate thread as suggested.
>
> ~
>
> PSA patch v2
Overall the patch looks good. But with so many 'relids' in play, it
takes time to figure out what each one represents. Do you think we
should also add below in the new comment:
/*
* relids holds every relation added to the result so far, both those the
* user named and inheritance children added implicitly by recursion. It
* is used to filter out duplicates.
*
* relids_explicit holds only the relations the user actually named (a
* subset of relids). relids_recursing holds those explicitly named
* relations whose first mention did not specify ONLY. Together they are
* used to detect conflicting ONLY specifications for the same table.
*/
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Eisentraut | 2026-10-19 07:20:47 | Re: Use C11 alignas instead of palloc/malloc for alignment |
| Previous Message | Peter Eisentraut | 2026-10-19 07:09:38 | Re: fix more casting away of qualifiers |