public inbox for [email protected]  

help / color / mirror / Atom feed

From: Shinya Kato <[email protected]>
To: jian he <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: remove unnecessary include in src/backend/commands/policy.c
Date: Tue, 30 Sep 2025 22:00:13 +0900
Message-ID: <CAOzEurQ9kYGd50j4O2RVT2FrLhESDCz8f+P+0y3eS1uYCJmSSg@mail.gmail.com> (raw)
In-Reply-To: <CACJufxFvcqOd6g6uaQqKuKPRgcEfPwp_tLSaaxDiHFBb2snJDA@mail.gmail.com>
References: <CACJufxFvcqOd6g6uaQqKuKPRgcEfPwp_tLSaaxDiHFBb2snJDA@mail.gmail.com>

Hi,

On Sun, Sep 14, 2025 at 3:38 PM jian he <[email protected]> wrote:
>
> hi.
>
> in src/backend/commands/policy.c, i found that
> #include "access/htup.h"
> #include "access/htup_details.h"
> #include "catalog/catalog.h"
> #include "nodes/pg_list.h"
> #include "parser/parse_node.h"
> #include "utils/array.h"
>
> is not necessary "include", so I removed it.
>
> we can also remove
> #include "access/relation.h"
> replace relation_open to table_open, since we already did relkind check in
> RangeVarCallbackForPolicy.

Thanks for the patch!
I can confirm that it builds successfully and passes the regression tests.

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.

-- 
Best regards,
Shinya Kato
NTT OSS Center

view thread (11+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: remove unnecessary include in src/backend/commands/policy.c
  In-Reply-To: <CAOzEurQ9kYGd50j4O2RVT2FrLhESDCz8f+P+0y3eS1uYCJmSSg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox