public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v5 13/16] wip: don't run ldap tests on windows.
11+ messages / 6 participants
[nested] [flat]

* [PATCH v3 14/17] wip: don't run ldap tests on windows.
@ 2021-10-10 20:49 Andres Freund <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Andres Freund @ 2021-10-10 20:49 UTC (permalink / raw)

---
 src/test/ldap/t/001_auth.pl | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index a025a641b02..c921a3d4872 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -6,6 +6,13 @@ use warnings;
 use TestLib;
 use PostgresNode;
 use Test::More;
+use Config;
+
+if ($Config{osname} eq 'MSWin32')
+{
+	plan skip_all => 'ldap tests ';
+	exit;
+}
 
 if ($ENV{with_ldap} eq 'yes')
 {
-- 
2.23.0.385.gbc12974a89


--qozqt6hocm2wibt2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0015-wip-split-TESTDIR-into-two.patch"



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* [PATCH v5 13/16] wip: don't run ldap tests on windows.
@ 2021-10-10 20:49 Andres Freund <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Andres Freund @ 2021-10-10 20:49 UTC (permalink / raw)

---
 src/test/ldap/t/001_auth.pl | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index 5a9a0098327..8850fc1cb1b 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -6,6 +6,13 @@ use warnings;
 use PostgreSQL::Test::Utils;
 use PostgreSQL::Test::Cluster;
 use Test::More;
+use Config;
+
+if ($Config{osname} eq 'MSWin32')
+{
+	plan skip_all => 'ldap tests ';
+	exit;
+}
 
 if ($ENV{with_ldap} eq 'yes')
 {
-- 
2.23.0.385.gbc12974a89


--q7iu5jwpuq7sbl3z
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0014-wip-split-TESTDIR-into-two.patch"



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* remove unnecessary include in src/backend/commands/policy.c
@ 2025-09-14 06:37 jian he <[email protected]>
  2025-09-15 00:18 ` Re: remove unnecessary include in src/backend/commands/policy.c Chao Li <[email protected]>
  2025-09-30 13:00 ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  0 siblings, 2 replies; 11+ messages in thread

From: jian he @ 2025-09-14 06:37 UTC (permalink / raw)
  To: pgsql-hackers

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.


Attachments:

  [text/x-patch] v1-0001-remove-unnecessary-include-in-policy.c.patch (3.0K, ../../CACJufxFvcqOd6g6uaQqKuKPRgcEfPwp_tLSaaxDiHFBb2snJDA@mail.gmail.com/2-v1-0001-remove-unnecessary-include-in-policy.c.patch)
  download | inline diff:
From 5ba1586d0844ec40674338ad920b12a61dd646ed Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Sun, 14 Sep 2025 14:35:33 +0800
Subject: [PATCH v1 1/1] remove unnecessary include in policy.c

discussion: https://postgr.es/m/
---
 src/backend/commands/policy.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 83056960fe4..ce3eb10ae86 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -13,12 +13,8 @@
 #include "postgres.h"
 
 #include "access/genam.h"
-#include "access/htup.h"
-#include "access/htup_details.h"
-#include "access/relation.h"
 #include "access/table.h"
 #include "access/xact.h"
-#include "catalog/catalog.h"
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
@@ -28,15 +24,12 @@
 #include "catalog/pg_type.h"
 #include "commands/policy.h"
 #include "miscadmin.h"
-#include "nodes/pg_list.h"
 #include "parser/parse_clause.h"
 #include "parser/parse_collate.h"
-#include "parser/parse_node.h"
 #include "parser/parse_relation.h"
 #include "rewrite/rewriteManip.h"
 #include "rewrite/rowsecurity.h"
 #include "utils/acl.h"
-#include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
 #include "utils/inval.h"
@@ -630,7 +623,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 +745,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 +798,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 +1075,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 +1103,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 +1182,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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: remove unnecessary include in src/backend/commands/policy.c
  2025-09-14 06:37 remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
@ 2025-09-15 00:18 ` Chao Li <[email protected]>
  1 sibling, 0 replies; 11+ messages in thread

From: Chao Li @ 2025-09-15 00:18 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: pgsql-hackers



> On Sep 14, 2025, at 14:37, 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.
> <v1-0001-remove-unnecessary-include-in-policy.c.patch>


LGTM. I built the patch on MacOS M4, and build passed without warning.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: remove unnecessary include in src/backend/commands/policy.c
  2025-09-14 06:37 remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
@ 2025-09-30 13:00 ` Shinya Kato <[email protected]>
  2025-10-12 08:31   ` Re: remove unnecessary include in src/backend/commands/policy.c Álvaro Herrera <[email protected]>
  1 sibling, 1 reply; 11+ messages in thread

From: Shinya Kato @ 2025-09-30 13:00 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: pgsql-hackers

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





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: remove unnecessary include in src/backend/commands/policy.c
  2025-09-14 06:37 remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
  2025-09-30 13:00 ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
@ 2025-10-12 08:31   ` Álvaro Herrera <[email protected]>
  2025-10-15 05:43     ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Álvaro Herrera @ 2025-10-12 08:31 UTC (permalink / raw)
  To: Shinya Kato <[email protected]>; +Cc: jian he <[email protected]>; pgsql-hackers

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.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Doing what he did amounts to sticking his fingers under the hood of the
implementation; if he gets his fingers burnt, it's his problem."  (Tom Lane)





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: remove unnecessary include in src/backend/commands/policy.c
  2025-09-14 06:37 remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
  2025-09-30 13:00 ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  2025-10-12 08:31   ` Re: remove unnecessary include in src/backend/commands/policy.c Álvaro Herrera <[email protected]>
@ 2025-10-15 05:43     ` Shinya Kato <[email protected]>
  2025-10-21 04:00       ` Re: remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Shinya Kato @ 2025-10-15 05:43 UTC (permalink / raw)
  To: Álvaro Herrera <[email protected]>; +Cc: jian he <[email protected]>; pgsql-hackers

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.

-- 
Best regards,
Shinya Kato
NTT OSS Center





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: remove unnecessary include in src/backend/commands/policy.c
  2025-09-14 06:37 remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
  2025-09-30 13:00 ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  2025-10-12 08:31   ` Re: remove unnecessary include in src/backend/commands/policy.c Álvaro Herrera <[email protected]>
  2025-10-15 05:43     ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
@ 2025-10-21 04:00       ` jian he <[email protected]>
  2025-10-22 06:30         ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: jian he @ 2025-10-21 04:00 UTC (permalink / raw)
  To: Shinya Kato <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; pgsql-hackers

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.


Attachments:

  [text/x-patch] v2-0001-rename-relation_open-close-to-table_open-close-in-policy.c.patch (2.4K, ../../CACJufxGTJrH6ZSpMMBOpAPhPy_Uirz7wpqgO8_Yki_rGqWviHw@mail.gmail.com/2-v2-0001-rename-relation_open-close-to-table_open-close-in-policy.c.patch)
  download | inline diff:
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_tLSaaxDiHFBb2snJDA@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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: remove unnecessary include in src/backend/commands/policy.c
  2025-09-14 06:37 remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
  2025-09-30 13:00 ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  2025-10-12 08:31   ` Re: remove unnecessary include in src/backend/commands/policy.c Álvaro Herrera <[email protected]>
  2025-10-15 05:43     ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  2025-10-21 04:00       ` Re: remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
@ 2025-10-22 06:30         ` Shinya Kato <[email protected]>
  2025-12-26 07:30           ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Shinya Kato @ 2025-10-22 06:30 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; pgsql-hackers

On Tue, Oct 21, 2025 at 1:01 PM jian he <[email protected]> wrote:
> > 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.

Thank you for updating the patch.
But I said "except for access/relation.h". I think access/relation.h
is not necessary if you replace relation_open to table_open.


-- 
Best regards,
Shinya Kato
NTT OSS Center





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: remove unnecessary include in src/backend/commands/policy.c
  2025-09-14 06:37 remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
  2025-09-30 13:00 ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  2025-10-12 08:31   ` Re: remove unnecessary include in src/backend/commands/policy.c Álvaro Herrera <[email protected]>
  2025-10-15 05:43     ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  2025-10-21 04:00       ` Re: remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
  2025-10-22 06:30         ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
@ 2025-12-26 07:30           ` Shinya Kato <[email protected]>
  2025-12-26 09:27             ` Re: remove unnecessary include in src/backend/commands/policy.c Japin Li <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Shinya Kato @ 2025-12-26 07:30 UTC (permalink / raw)
  To: jian he <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; pgsql-hackers

On Wed, Oct 22, 2025 at 3:30 PM Shinya Kato <[email protected]> wrote:
> Thank you for updating the patch.
> But I said "except for access/relation.h". I think access/relation.h
> is not necessary if you replace relation_open to table_open.

I've fixed it, and marked the patch as Ready for Committer.

-- 
Best regards,
Shinya Kato
NTT OSS Center


Attachments:

  [application/octet-stream] v3-0001-Replace-relation_-open-close-to-table_-open-close.patch (2.9K, ../../CAOzEurQFPH+UNk17MbDhupLCm-i5Gf9dJ4Kx2-er=-YYV4r+sQ@mail.gmail.com/2-v3-0001-Replace-relation_-open-close-to-table_-open-close.patch)
  download | inline diff:
From 4b529266ebf18f95ca4e8c335d2a494812cf65bd Mon Sep 17 00:00:00 2001
From: Shinya Kato <[email protected]>
Date: Fri, 26 Dec 2025 16:19:26 +0900
Subject: [PATCH v3] Replace relation_{open,close} to table_{open,close} in
 policy.c
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RangeVarCallbackForPolicy already ensures policies apply only to tables
or partitioned tables, so table_* isthe appropriate API for
opening/closing the target relation.

Author: Jian He <[email protected]>
Reviewed-by: Chao Li <[email protected]>
Reviewed-by: Shinya Kato <[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/CACJufxFvcqOd6g6uaQqKuKPRgcEfPwp_tLSaaxDiHFBb2snJDA@mail.gmail.com
---
 src/backend/commands/policy.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 5bd5f8c9968..b75592434b8 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -15,7 +15,6 @@
 #include "access/genam.h"
 #include "access/htup.h"
 #include "access/htup_details.h"
-#include "access/relation.h"
 #include "access/table.h"
 #include "access/xact.h"
 #include "catalog/catalog.h"
@@ -630,7 +629,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 +751,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 +804,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 +1081,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 +1109,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 +1188,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.47.3



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: remove unnecessary include in src/backend/commands/policy.c
  2025-09-14 06:37 remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
  2025-09-30 13:00 ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  2025-10-12 08:31   ` Re: remove unnecessary include in src/backend/commands/policy.c Álvaro Herrera <[email protected]>
  2025-10-15 05:43     ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  2025-10-21 04:00       ` Re: remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
  2025-10-22 06:30         ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
  2025-12-26 07:30           ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
@ 2025-12-26 09:27             ` Japin Li <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Japin Li @ 2025-12-26 09:27 UTC (permalink / raw)
  To: Shinya Kato <[email protected]>; +Cc: jian he <[email protected]>; Álvaro Herrera <[email protected]>; pgsql-hackers


Hi, Shinya

On Fri, 26 Dec 2025 at 16:30, Shinya Kato <[email protected]> wrote:
> On Wed, Oct 22, 2025 at 3:30 PM Shinya Kato <[email protected]> wrote:
>> Thank you for updating the patch.
>> But I said "except for access/relation.h". I think access/relation.h
>> is not necessary if you replace relation_open to table_open.
>
> I've fixed it, and marked the patch as Ready for Committer.
>

Found a typo in the commit message.

> RangeVarCallbackForPolicy already ensures policies apply only to tables
> or partitioned tables, so table_* isthe appropriate API for
> opening/closing the target relation.

s/isthe/is the/


-- 
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.





^ permalink  raw  reply  [nested|flat] 11+ messages in thread


end of thread, other threads:[~2025-12-26 09:27 UTC | newest]

Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-10-10 20:49 [PATCH v3 14/17] wip: don't run ldap tests on windows. Andres Freund <[email protected]>
2021-10-10 20:49 [PATCH v5 13/16] wip: don't run ldap tests on windows. Andres Freund <[email protected]>
2025-09-14 06:37 remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
2025-09-15 00:18 ` Re: remove unnecessary include in src/backend/commands/policy.c Chao Li <[email protected]>
2025-09-30 13:00 ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
2025-10-12 08:31   ` Re: remove unnecessary include in src/backend/commands/policy.c Álvaro Herrera <[email protected]>
2025-10-15 05:43     ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
2025-10-21 04:00       ` Re: remove unnecessary include in src/backend/commands/policy.c jian he <[email protected]>
2025-10-22 06:30         ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
2025-12-26 07:30           ` Re: remove unnecessary include in src/backend/commands/policy.c Shinya Kato <[email protected]>
2025-12-26 09:27             ` Re: remove unnecessary include in src/backend/commands/policy.c Japin Li <[email protected]>

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