On Wed, Oct 15, 2025 at 1:44 PM Shinya Kato <[email protected]> wrote: > > On Sun, Oct 12, 2025 at 5:31 PM Álvaro Herrera <[email protected]> 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.
From 766951113b6df59e5fa393b57487e3d2e42dc027 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Tue, 21 Oct 2025 11:59:36 +0800 Subject: [PATCH v2 1/1] rename relation_open/close to table_open/close in policy.c RangeVarCallbackForPolicy already checks that a policy can only be created on a table or a partitioned table. discussion: https://postgr.es/m/cacjufxfvcqod6g6uaqqkukprgcefpwp_tlsaaxdihfbb2sn...@mail.gmail.com --- src/backend/commands/policy.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index 83056960fe4..9fec50422c4 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -630,7 +630,7 @@ CreatePolicy(CreatePolicyStmt *stmt) stmt); /* Open target_table to build quals. No additional lock is necessary. */ - target_table = relation_open(table_id, NoLock); + target_table = table_open(table_id, NoLock); /* Add for the regular security quals */ nsitem = addRangeTableEntryForRelation(qual_pstate, target_table, @@ -752,7 +752,7 @@ CreatePolicy(CreatePolicyStmt *stmt) free_parsestate(qual_pstate); free_parsestate(with_check_pstate); systable_endscan(sscan); - relation_close(target_table, NoLock); + table_close(target_table, NoLock); table_close(pg_policy_rel, RowExclusiveLock); return myself; @@ -805,7 +805,7 @@ AlterPolicy(AlterPolicyStmt *stmt) RangeVarCallbackForPolicy, stmt); - target_table = relation_open(table_id, NoLock); + target_table = table_open(table_id, NoLock); /* Parse the using policy clause */ if (stmt->qual) @@ -1082,7 +1082,7 @@ AlterPolicy(AlterPolicyStmt *stmt) /* Clean up. */ systable_endscan(sscan); - relation_close(target_table, NoLock); + table_close(target_table, NoLock); table_close(pg_policy_rel, RowExclusiveLock); return myself; @@ -1110,7 +1110,7 @@ rename_policy(RenameStmt *stmt) RangeVarCallbackForPolicy, stmt); - target_table = relation_open(table_id, NoLock); + target_table = table_open(table_id, NoLock); pg_policy_rel = table_open(PolicyRelationId, RowExclusiveLock); @@ -1189,7 +1189,7 @@ rename_policy(RenameStmt *stmt) /* Clean up. */ systable_endscan(sscan); table_close(pg_policy_rel, RowExclusiveLock); - relation_close(target_table, NoLock); + table_close(target_table, NoLock); return address; } -- 2.34.1
