public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v3 14/17] wip: don't run ldap tests on windows.
12+ messages / 7 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; 12+ 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] 12+ 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; 12+ 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] 12+ messages in thread
* Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c)
@ 2024-06-24 11:37 Ranier Vilela <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Ranier Vilela @ 2024-06-24 11:37 UTC (permalink / raw)
To: Yugo NAGATA <[email protected]>; +Cc: Michael Paquier <[email protected]>; Fabrízio de Royes Mello <[email protected]>; pgsql-hackers
Em seg., 24 de jun. de 2024 às 00:27, Yugo NAGATA <[email protected]>
escreveu:
> On Sun, 23 Jun 2024 22:34:03 -0300
> Ranier Vilela <[email protected]> wrote:
>
> > Em dom., 23 de jun. de 2024 às 22:14, Ranier Vilela <[email protected]
> >
> > escreveu:
> >
> > > Em dom., 23 de jun. de 2024 às 22:05, Ranier Vilela <
> [email protected]>
> > > escreveu:
> > >
> > >> Em dom., 23 de jun. de 2024 às 21:54, Michael Paquier <
> > >> [email protected]> escreveu:
> > >>
> > >>> On Sun, Jun 23, 2024 at 09:34:45PM -0300, Ranier Vilela wrote:
> > >>> > It's not critical code, so I think it's ok to use strlen, even
> because
> > >>> the
> > >>> > result of strlen will already be available using modern compilers.
> > >>> >
> > >>> > So, I think it's ok to use memcpy with strlen + 1.
> > >>>
> > >>> It seems to me that there is a pretty good argument to just use
> > >>> strlcpy() for the same reason as the one you cite: this is not a
> > >>> performance-critical code, and that's just safer.
> > >>>
> > >> Yeah, I'm fine with strlcpy. I'm not against it.
> > >>
> > > Perhaps, like the v2?
> > >
> > > Either v1 or v2, to me, looks good.
> > >
> > Thinking about, does not make sense the field size MAXPGPATH + 1.
> > all other similar fields are just MAXPGPATH.
> >
> > If we copy MAXPGPATH + 1, it will also be wrong.
> > So it is necessary to adjust logbackup.h as well.
>
> I am not sure whether we need to change the size of the field,
> but if change it, I wonder it is better to modify the following
> message from MAXPGPATH to MAXPGPATH -1.
>
> errmsg("backup label too long (max %d
> bytes)",
> MAXPGPATH)));
>
Or perhaps, is it better to show the too long label?
errmsg("backup label too long (%s)",
backupidstr)));
best regards,
Ranier Vilela
>
> >
> > So, I think that v3 is ok to fix.
> >
> > best regards,
> > Ranier Vilela
> >
> > >
> > > best regards,
> > > Ranier Vilela
> > >
> > >>
>
>
> --
> Yugo NAGATA <[email protected]>
>
^ permalink raw reply [nested|flat] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2025-12-26 09:27 UTC | newest]
Thread overview: 12+ 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]>
2024-06-24 11:37 Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c) Ranier Vilela <[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