Re: remove unnecessary include in src/backend/commands/policy.c

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Shinya Kato <shinya11(dot)kato(at)gmail(dot)com>
Cc: Álvaro Herrera <alvherre(at)kurilemu(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: remove unnecessary include in src/backend/commands/policy.c
Date: 2025-10-21 04:00:28
Message-ID: CACJufxGTJrH6ZSpMMBOpAPhPy_Uirz7wpqgO8_Yki_rGqWviHw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Oct 15, 2025 at 1:44 PM Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> wrote:
>
> On Sun, Oct 12, 2025 at 5:31 PM Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
> >
> > On 2025-Sep-30, Shinya Kato wrote:
> >
> > > However, the changes make policy.c rely on transitive includes. For
> > > example, policy.c uses GETSTRUCT(), which is defined in
> > > access/htup_details.h. Instead of being included directly, that header
> > > is currently pulled in via a fairly long chain:
> > > catalog/indexing.h -> nodes/execnodes.h -> access/tupconvert.h ->
> > > executor/tuptable.h -> access/htup_details.h
> > >
> > > While this works for now, the dependency is fragile and could break if
> > > header files are rearranged in the future. I'm not sure this is a good
> > > practice, and although I couldn't find a specific rule against it in
> > > PostgreSQL's coding conventions, it seems risky.
> >
> > Yeah -- I'm not very worried about the fragility being introduced, since
> > if such a problem ever occurs it's very obvious and easy to fix.
> > However, removing these include lines is just churn with no benefit,
> > because those includes are still being processed via the indirect
> > pathways. We haven't saved anything.
> >
> > Just look at all the crossed wires here
> > https://doxygen.postgresql.org/policy_8c.html
> > Clearly the cross-inclusion of headers in headers is a mess. Fixing
> > that mess is going to cause *more* explicit inclusion of headers in .c
> > files. Removing a few explicit ones so that they become implicit, only
> > to have to resurrect the explicit inclusion when we remove some of that
> > cross-header inclusion is pointless.
>
> Thank you, I agree with Álvaro. So, I think it is better to leave them
> as they are, except for access/relation.h. And, replacing
> relation_open to table_open looks good to me.
>

ok.

The attached patch only replaces relation_open to table_open.
RangeVarCallbackForPolicy already checks that a policy can only be created on a
table or a partitioned table.

so the replacement should be ok.

Attachment Content-Type Size
v2-0001-rename-relation_open-close-to-table_open-close-in-policy.c.patch text/x-patch 2.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2025-10-21 04:03:29 Re: Logical Replication of sequences
Previous Message Richard Guo 2025-10-21 03:58:53 Re: Inconsistent Behavior of GROUP BY ROLLUP in v17 vs master