pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedAdding missing object access hook invocations
18+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2020-05-23 a995b371ae Add missing invocations to object access hooks
* Adding missing object access hook invocations
@ 2020-03-16 23:03 Mark Dilger <mark.dilger@enterprisedb.com>
0 siblings, 2 replies; 18+ messages in thread
From: Mark Dilger @ 2020-03-16 23:03 UTC (permalink / raw)
To: pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
Hackers,
While working on object access hooks, I noticed several locations where I would expect the hook to be invoked, but no actual invocation. I think this just barely qualifies as a bug. It's debatable because whether it is a bug depends on the user's expectations and whether not invoking the hook in these cases is defensible. Does anybody have any recollection of an intentional choice not to invoke in these locations?
Patch attached.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
v1-0001-Adding-missing-Object-Access-hook-invocations.patch (application/octet-stream, 3.5K) download
From 2759f8deefc89e40eb6a299540961aa5e8fd6b61 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Mon, 16 Mar 2020 08:53:32 -0700
Subject: [PATCH v1] Adding missing Object Access hook invocations.
There appears to be no reason for skipping the invocation of the
object access hook infrastructure for the following commands, yet
they were lacking the InvokeObject{PostCreate,PostAlter,Drop}Hook
calls that would be expected:
ALTER RULE
ALTER USER MAPPING
CREATE ACCESS METHOD
CREATE STATISTICS
DROP STATISTICS
DROP ACCESS METHOD
There doesn't seem to be any good regression test coverage for
when and how the object_access_hook is invoked, so no additional
coverage is included here, either. Adding regression test
coverage seems like another patch to be submitted separately.
---
src/backend/commands/amcmds.c | 5 +++++
src/backend/commands/foreigncmds.c | 3 +++
src/backend/commands/statscmds.c | 4 ++++
src/backend/rewrite/rewriteDefine.c | 2 ++
4 files changed, 14 insertions(+)
diff --git a/src/backend/commands/amcmds.c b/src/backend/commands/amcmds.c
index 7546378bbb..1ecad090ee 100644
--- a/src/backend/commands/amcmds.c
+++ b/src/backend/commands/amcmds.c
@@ -18,6 +18,7 @@
#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
+#include "catalog/objectaccess.h"
#include "catalog/pg_am.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
@@ -107,6 +108,8 @@ CreateAccessMethod(CreateAmStmt *stmt)
recordDependencyOnCurrentExtension(&myself, false);
+ InvokeObjectPostCreateHook(AccessMethodRelationId, amoid, 0);
+
table_close(rel, RowExclusiveLock);
return myself;
@@ -134,6 +137,8 @@ RemoveAccessMethodById(Oid amOid)
CatalogTupleDelete(relation, &tup->t_self);
+ InvokeObjectDropHook(AccessMethodRelationId, amOid, 0);
+
ReleaseSysCache(tup);
table_close(relation, RowExclusiveLock);
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index f197869752..a399ab4de9 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -1343,6 +1343,9 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
CatalogTupleUpdate(rel, &tp->t_self, tp);
+ InvokeObjectPostAlterHook(UserMappingRelationId,
+ umId, 0);
+
ObjectAddressSet(address, UserMappingRelationId, umId);
heap_freetuple(tp);
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 988cdba6f5..08d0da1109 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -374,6 +374,8 @@ CreateStatistics(CreateStatsStmt *stmt)
relation_close(datarel, RowExclusiveLock);
+ InvokeObjectPostCreateHook(StatisticExtRelationId, statoid, 0);
+
/*
* Invalidate relcache so that others see the new statistics object.
*/
@@ -568,6 +570,8 @@ RemoveStatisticsById(Oid statsOid)
CatalogTupleDelete(relation, &tup->t_self);
+ InvokeObjectDropHook(StatisticExtRelationId, statsOid, 0);
+
ReleaseSysCache(tup);
table_close(relation, RowExclusiveLock);
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index afc78b3316..9989df1107 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -1003,6 +1003,8 @@ RenameRewriteRule(RangeVar *relation, const char *oldName,
CatalogTupleUpdate(pg_rewrite_desc, &ruletup->t_self, ruletup);
+ InvokeObjectPostAlterHook(RewriteRelationId, ruleOid, 0);
+
heap_freetuple(ruletup);
table_close(pg_rewrite_desc, RowExclusiveLock);
--
2.21.1 (Apple Git-122.3)
=
* Re: Adding missing object access hook invocations
@ 2020-03-17 00:14 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Mark Dilger <mark.dilger@enterprisedb.com>
1 sibling, 1 reply; 18+ messages in thread
From: Alvaro Herrera @ 2020-03-17 00:14 UTC (permalink / raw)
To: Mark Dilger <mark.dilger@enterprisedb.com>; +Cc: pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>; Kohei KaiGai <kaigai@heterodb.com>; Adam Brightwell <adam.brightwell@crunchydata.com>
On 2020-Mar-16, Mark Dilger wrote:
> Hackers,
>
> While working on object access hooks, I noticed several locations where I would expect the hook to be invoked, but no actual invocation. I think this just barely qualifies as a bug. It's debatable because whether it is a bug depends on the user's expectations and whether not invoking the hook in these cases is defensible. Does anybody have any recollection of an intentional choice not to invoke in these locations?
Hmm, possibly the create-time calls are missing.
I'm surprised about the InvokeObjectDropHook calls though. Doesn't
deleteOneObject already call that? If we have more calls elsewhere,
maybe they are redundant. I think we should only have those for
"shared" objects.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-03-17 00:58 Mark Dilger <mark.dilger@enterprisedb.com>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 0 replies; 18+ messages in thread
From: Mark Dilger @ 2020-03-17 00:58 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>; Kohei KaiGai <kaigai@heterodb.com>; Adam Brightwell <adam.brightwell@crunchydata.com>
> On Mar 16, 2020, at 5:14 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> On 2020-Mar-16, Mark Dilger wrote:
>
>> Hackers,
>>
>> While working on object access hooks, I noticed several locations where I would expect the hook to be invoked, but no actual invocation. I think this just barely qualifies as a bug. It's debatable because whether it is a bug depends on the user's expectations and whether not invoking the hook in these cases is defensible. Does anybody have any recollection of an intentional choice not to invoke in these locations?
>
> Hmm, possibly the create-time calls are missing.
It looks to me that both the create and alter calls are missing.
>
> I'm surprised about the InvokeObjectDropHook calls though. Doesn't
> deleteOneObject already call that? If we have more calls elsewhere,
> maybe they are redundant. I think we should only have those for
> "shared" objects.
Yeah, you are right about the drop hook being invoked elsewhere for dropping ACCESS METHOD and STATISTICS. Sorry for the noise.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-03-17 18:49 Andres Freund <andres@anarazel.de>
parent: Mark Dilger <mark.dilger@enterprisedb.com>
1 sibling, 1 reply; 18+ messages in thread
From: Andres Freund @ 2020-03-17 18:49 UTC (permalink / raw)
To: Mark Dilger <mark.dilger@enterprisedb.com>; +Cc: pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
Hi,
On 2020-03-16 16:03:51 -0700, Mark Dilger wrote:
> While working on object access hooks, I noticed several locations
> where I would expect the hook to be invoked, but no actual invocation.
> I think this just barely qualifies as a bug. It's debatable because
> whether it is a bug depends on the user's expectations and whether not
> invoking the hook in these cases is defensible. Does anybody have any
> recollection of an intentional choice not to invoke in these
> locations?
I am strongly against treating this as a bug, which'd likely imply
backpatching. New hook invocations are a noticable behavioural change,
and very plausibly will break currently working extensions. That's fine
for a major version upgrade, but not for a minor one, unless there are
very good reasons.
Andres
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-03-17 19:39 Mark Dilger <mark.dilger@enterprisedb.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 18+ messages in thread
From: Mark Dilger @ 2020-03-17 19:39 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
> On Mar 17, 2020, at 11:49 AM, Andres Freund <andres@anarazel.de> wrote:
>
> On 2020-03-16 16:03:51 -0700, Mark Dilger wrote:
>> While working on object access hooks, I noticed several locations
>> where I would expect the hook to be invoked, but no actual invocation.
>> I think this just barely qualifies as a bug. It's debatable because
>> whether it is a bug depends on the user's expectations and whether not
>> invoking the hook in these cases is defensible. Does anybody have any
>> recollection of an intentional choice not to invoke in these
>> locations?
>
> I am strongly against treating this as a bug, which'd likely imply
> backpatching. New hook invocations are a noticable behavioural change,
> and very plausibly will break currently working extensions. That's fine
> for a major version upgrade, but not for a minor one, unless there are
> very good reasons.
I agree that this does not need to be back-patched. I was debating whether it constitutes a bug for the purpose of putting the fix into v13 vs. punting the patch forward to the v14 cycle. I don't have a strong opinion on that.
Thoughts?
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-03-18 04:33 Michael Paquier <michael@paquier.xyz>
parent: Mark Dilger <mark.dilger@enterprisedb.com>
0 siblings, 1 reply; 18+ messages in thread
From: Michael Paquier @ 2020-03-18 04:33 UTC (permalink / raw)
To: Mark Dilger <mark.dilger@enterprisedb.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
On Tue, Mar 17, 2020 at 12:39:35PM -0700, Mark Dilger wrote:
> I agree that this does not need to be back-patched. I was debating
> whether it constitutes a bug for the purpose of putting the fix into
> v13 vs. punting the patch forward to the v14 cycle. I don't have a
> strong opinion on that.
I don't see any strong argument against fixing this stuff in v13,
FWIW.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-03-18 14:50 Mark Dilger <mark.dilger@enterprisedb.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 18+ messages in thread
From: Mark Dilger @ 2020-03-18 14:50 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
> On Mar 17, 2020, at 9:33 PM, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Mar 17, 2020 at 12:39:35PM -0700, Mark Dilger wrote:
>> I agree that this does not need to be back-patched. I was debating
>> whether it constitutes a bug for the purpose of putting the fix into
>> v13 vs. punting the patch forward to the v14 cycle. I don't have a
>> strong opinion on that.
>
> I don't see any strong argument against fixing this stuff in v13,
> FWIW.
Here is the latest patch. I'll go add this thread to the commitfest app now....
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
v2-0001-Adding-missing-Object-Access-hook-invocations.patch (application/octet-stream, 3.5K) download
From 2759f8deefc89e40eb6a299540961aa5e8fd6b61 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Mon, 16 Mar 2020 08:53:32 -0700
Subject: [PATCH v2] Adding missing Object Access hook invocations.
There appears to be no reason for skipping the invocation of the object
access hook infrastructure for the following commands, yet they were
lacking the InvokeObjectPostCreateHook and InvokeObjectPostAlterHook
calls that would be expected:
ALTER RULE
ALTER USER MAPPING
CREATE ACCESS METHOD
CREATE STATISTICS
There doesn't seem to be any good regression test coverage for
when and how the object_access_hook is invoked, so no additional
coverage is included here, either. Adding regression test
coverage seems like another patch to be submitted separately.
---
src/backend/commands/amcmds.c | 5 +++++
src/backend/commands/foreigncmds.c | 3 +++
src/backend/commands/statscmds.c | 4 ++++
src/backend/rewrite/rewriteDefine.c | 2 ++
4 files changed, 14 insertions(+)
diff --git a/src/backend/commands/amcmds.c b/src/backend/commands/amcmds.c
index 7546378bbb..1ecad090ee 100644
--- a/src/backend/commands/amcmds.c
+++ b/src/backend/commands/amcmds.c
@@ -18,6 +18,7 @@
#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
+#include "catalog/objectaccess.h"
#include "catalog/pg_am.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
@@ -107,6 +108,8 @@ CreateAccessMethod(CreateAmStmt *stmt)
recordDependencyOnCurrentExtension(&myself, false);
+ InvokeObjectPostCreateHook(AccessMethodRelationId, amoid, 0);
+
table_close(rel, RowExclusiveLock);
return myself;
@@ -134,6 +137,8 @@ RemoveAccessMethodById(Oid amOid)
CatalogTupleDelete(relation, &tup->t_self);
+ InvokeObjectDropHook(AccessMethodRelationId, amOid, 0);
+
ReleaseSysCache(tup);
table_close(relation, RowExclusiveLock);
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index f197869752..a399ab4de9 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -1343,6 +1343,9 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
CatalogTupleUpdate(rel, &tp->t_self, tp);
+ InvokeObjectPostAlterHook(UserMappingRelationId,
+ umId, 0);
+
ObjectAddressSet(address, UserMappingRelationId, umId);
heap_freetuple(tp);
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 988cdba6f5..08d0da1109 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -374,6 +374,8 @@ CreateStatistics(CreateStatsStmt *stmt)
relation_close(datarel, RowExclusiveLock);
+ InvokeObjectPostCreateHook(StatisticExtRelationId, statoid, 0);
+
/*
* Invalidate relcache so that others see the new statistics object.
*/
@@ -568,6 +570,8 @@ RemoveStatisticsById(Oid statsOid)
CatalogTupleDelete(relation, &tup->t_self);
+ InvokeObjectDropHook(StatisticExtRelationId, statsOid, 0);
+
ReleaseSysCache(tup);
table_close(relation, RowExclusiveLock);
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index afc78b3316..9989df1107 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -1003,6 +1003,8 @@ RenameRewriteRule(RangeVar *relation, const char *oldName,
CatalogTupleUpdate(pg_rewrite_desc, &ruletup->t_self, ruletup);
+ InvokeObjectPostAlterHook(RewriteRelationId, ruleOid, 0);
+
heap_freetuple(ruletup);
table_close(pg_rewrite_desc, RowExclusiveLock);
--
2.21.1 (Apple Git-122.3)
=
* Re: Adding missing object access hook invocations
@ 2020-03-19 18:17 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Mark Dilger <mark.dilger@enterprisedb.com>
0 siblings, 1 reply; 18+ messages in thread
From: Alvaro Herrera @ 2020-03-19 18:17 UTC (permalink / raw)
To: Mark Dilger <mark.dilger@enterprisedb.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
On 2020-Mar-18, Mark Dilger wrote:
> Here is the latest patch.
So you insist in keeping the Drop hook calls?
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-03-19 18:30 Mark Dilger <mark.dilger@enterprisedb.com>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 18+ messages in thread
From: Mark Dilger @ 2020-03-19 18:30 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
> On Mar 19, 2020, at 11:17 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> On 2020-Mar-18, Mark Dilger wrote:
>
>> Here is the latest patch.
>
> So you insist in keeping the Drop hook calls?
My apologies, not at all. I appear to have attached the wrong patch. Will post v3 shortly.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-03-19 18:47 Mark Dilger <mark.dilger@enterprisedb.com>
parent: Mark Dilger <mark.dilger@enterprisedb.com>
0 siblings, 1 reply; 18+ messages in thread
From: Mark Dilger @ 2020-03-19 18:47 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
> On Mar 19, 2020, at 11:30 AM, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>
> Will post v3 shortly.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
v3-0001-Adding-missing-Object-Access-hook-invocations.patch (application/octet-stream, 3.0K) download
From 90119a3ea6dfa6ca28d02c30dbdce370e8f0a3a6 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Thu, 19 Mar 2020 11:35:09 -0700
Subject: [PATCH v3] Adding missing Object Access hook invocations.
There appears to be no reason for skipping the invocation of the
object access hook infrastructure for the following commands, yet they
were lacking the InvokeObjectPost{Create,Alter}Hook calls that would
be expected:
ALTER RULE
ALTER USER MAPPING
CREATE ACCESS METHOD
CREATE STATISTICS
There doesn't seem to be any good regression test coverage for when
and how the object_access_hook is invoked, so no additional coverage
is included here, either. Adding regression test coverage seems like
another patch to be submitted separately.
---
src/backend/commands/amcmds.c | 3 +++
src/backend/commands/foreigncmds.c | 3 +++
src/backend/commands/statscmds.c | 2 ++
src/backend/rewrite/rewriteDefine.c | 2 ++
4 files changed, 10 insertions(+)
diff --git a/src/backend/commands/amcmds.c b/src/backend/commands/amcmds.c
index 7546378bbb..b884bfa0b0 100644
--- a/src/backend/commands/amcmds.c
+++ b/src/backend/commands/amcmds.c
@@ -18,6 +18,7 @@
#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
+#include "catalog/objectaccess.h"
#include "catalog/pg_am.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
@@ -107,6 +108,8 @@ CreateAccessMethod(CreateAmStmt *stmt)
recordDependencyOnCurrentExtension(&myself, false);
+ InvokeObjectPostCreateHook(AccessMethodRelationId, amoid, 0);
+
table_close(rel, RowExclusiveLock);
return myself;
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index f197869752..a399ab4de9 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -1343,6 +1343,9 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
CatalogTupleUpdate(rel, &tp->t_self, tp);
+ InvokeObjectPostAlterHook(UserMappingRelationId,
+ umId, 0);
+
ObjectAddressSet(address, UserMappingRelationId, umId);
heap_freetuple(tp);
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 988cdba6f5..d30059d043 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -374,6 +374,8 @@ CreateStatistics(CreateStatsStmt *stmt)
relation_close(datarel, RowExclusiveLock);
+ InvokeObjectPostCreateHook(StatisticExtRelationId, statoid, 0);
+
/*
* Invalidate relcache so that others see the new statistics object.
*/
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index afc78b3316..9989df1107 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -1003,6 +1003,8 @@ RenameRewriteRule(RangeVar *relation, const char *oldName,
CatalogTupleUpdate(pg_rewrite_desc, &ruletup->t_self, ruletup);
+ InvokeObjectPostAlterHook(RewriteRelationId, ruleOid, 0);
+
heap_freetuple(ruletup);
table_close(pg_rewrite_desc, RowExclusiveLock);
--
2.21.1 (Apple Git-122.3)
=
* Re: Adding missing object access hook invocations
@ 2020-04-19 22:55 Michael Paquier <michael@paquier.xyz>
parent: Mark Dilger <mark.dilger@enterprisedb.com>
0 siblings, 2 replies; 18+ messages in thread
From: Michael Paquier @ 2020-04-19 22:55 UTC (permalink / raw)
To: Mark Dilger <mark.dilger@enterprisedb.com>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
On Thu, Mar 19, 2020 at 11:47:46AM -0700, Mark Dilger wrote:
> On Mar 19, 2020, at 11:30 AM, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>> Will post v3 shortly.
Thanks for sending a new version of the patch and removing the bits
about object drops. Your additions to src/backend/ look fine to me,
so I have no objections to commit it. The only module we have in core
that makes use of object_access_hook is sepgsql. Adding support for
it could be done in a separate commit for AMs, stats and user mappings
but we would need a use-case for it. One thing that I can see is that
even if we test for ALTER put_your_object_type_here foo RENAME TO in
the module and that your patch adds one InvokeObjectPostAlterHook()
for ALTER RULE, we don't have support for rules in sepgsql (see
sepgsql_object_access for OAT_POST_CREATE). So that's fine.
Unfortunately, we are past feature freeze so this will have to wait
until v14 opens for business to be merged, and I'll take care of it.
Or would others prefer to not wait one extra year for those changes to
be released?
Please note that there is a commit fest entry, though you forgot to
add your name as author of the patch:
https://commitfest.postgresql.org/28/2513/
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-04-19 23:43 Mark Dilger <mark.dilger@enterprisedb.com>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 18+ messages in thread
From: Mark Dilger @ 2020-04-19 23:43 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
> On Apr 19, 2020, at 3:55 PM, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Mar 19, 2020 at 11:47:46AM -0700, Mark Dilger wrote:
>> On Mar 19, 2020, at 11:30 AM, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>>> Will post v3 shortly.
>
> Thanks for sending a new version of the patch and removing the bits
> about object drops. Your additions to src/backend/ look fine to me,
> so I have no objections to commit it. The only module we have in core
> that makes use of object_access_hook is sepgsql. Adding support for
> it could be done in a separate commit for AMs, stats and user mappings
> but we would need a use-case for it. One thing that I can see is that
> even if we test for ALTER put_your_object_type_here foo RENAME TO in
> the module and that your patch adds one InvokeObjectPostAlterHook()
> for ALTER RULE, we don't have support for rules in sepgsql (see
> sepgsql_object_access for OAT_POST_CREATE). So that's fine.
>
> Unfortunately, we are past feature freeze so this will have to wait
> until v14 opens for business to be merged, and I'll take care of it.
> Or would others prefer to not wait one extra year for those changes to
> be released?
I don't intend to make any special pleading for this to go in after feature freeze. Let's see if others feel differently.
Thanks for the review!
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-04-20 17:32 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 18+ messages in thread
From: Alvaro Herrera @ 2020-04-20 17:32 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Mark Dilger <mark.dilger@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
On 2020-Apr-20, Michael Paquier wrote:
> Unfortunately, we are past feature freeze so this will have to wait
> until v14 opens for business to be merged, and I'll take care of it.
> Or would others prefer to not wait one extra year for those changes to
> be released?
I think it's fine to put this in at this time. It's not a new feature.
The only thing this needs is to go through a new release cycle so that
people can adjust to the new hook invocations as necessary.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-04-21 01:39 Michael Paquier <michael@paquier.xyz>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 18+ messages in thread
From: Michael Paquier @ 2020-04-21 01:39 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Mark Dilger <mark.dilger@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
On Mon, Apr 20, 2020 at 01:32:31PM -0400, Alvaro Herrera wrote:
> I think it's fine to put this in at this time. It's not a new feature.
> The only thing this needs is to go through a new release cycle so that
> people can adjust to the new hook invocations as necessary.
Okay. Any other opinions? I am in a 50/50 state about that stuff.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-05-20 17:57 Robert Haas <robertmhaas@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 18+ messages in thread
From: Robert Haas @ 2020-05-20 17:57 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Mark Dilger <mark.dilger@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
On Mon, Apr 20, 2020 at 9:40 PM Michael Paquier <michael@paquier.xyz> wrote:
> Okay. Any other opinions? I am in a 50/50 state about that stuff.
I don't really see any reason why this couldn't be committed even at
this late date, but I also don't care that much. I suspect the number
of extension authors who are likely to have to make any code changes
is small. It's anybody's guess whether those people would like these
changes (because now they can support all of these object types even
in v13, rather than having to wait another year) or dislike them
(because it breaks something). I would actually be more inclined to
bet on the former rather than the latter, but unless somebody speaks
up, it's all just speculation.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-05-21 00:32 Michael Paquier <michael@paquier.xyz>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 18+ messages in thread
From: Michael Paquier @ 2020-05-21 00:32 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Mark Dilger <mark.dilger@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
On Wed, May 20, 2020 at 01:57:31PM -0400, Robert Haas wrote:
> I don't really see any reason why this couldn't be committed even at
> this late date, but I also don't care that much. I suspect the number
> of extension authors who are likely to have to make any code changes
> is small. It's anybody's guess whether those people would like these
> changes (because now they can support all of these object types even
> in v13, rather than having to wait another year) or dislike them
> (because it breaks something). I would actually be more inclined to
> bet on the former rather than the latter, but unless somebody speaks
> up, it's all just speculation.
Thanks for the input, Robert. So, even if we are post-beta1 it looks
like there are more upsides than downsides to get that stuff done
sooner than later. I propose to get that applied in the next couple
of days, please let me know if there are any objections.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-05-23 05:21 Michael Paquier <michael@paquier.xyz>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 18+ messages in thread
From: Michael Paquier @ 2020-05-23 05:21 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Mark Dilger <mark.dilger@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
On Thu, May 21, 2020 at 09:32:55AM +0900, Michael Paquier wrote:
> Thanks for the input, Robert. So, even if we are post-beta1 it looks
> like there are more upsides than downsides to get that stuff done
> sooner than later. I propose to get that applied in the next couple
> of days, please let me know if there are any objections.
Hearing nothing, done. Thanks all for the discussion.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Adding missing object access hook invocations
@ 2020-05-24 02:31 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 18+ messages in thread
From: Alvaro Herrera @ 2020-05-24 02:31 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Robert Haas <robertmhaas@gmail.com>; Mark Dilger <mark.dilger@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers@postgresql.org <pgsql-hackers@lists.postgresql.org>
On 2020-May-23, Michael Paquier wrote:
> On Thu, May 21, 2020 at 09:32:55AM +0900, Michael Paquier wrote:
> > Thanks for the input, Robert. So, even if we are post-beta1 it looks
> > like there are more upsides than downsides to get that stuff done
> > sooner than later. I propose to get that applied in the next couple
> > of days, please let me know if there are any objections.
>
> Hearing nothing, done. Thanks all for the discussion.
Thanks!
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2020-05-24 02:31 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-16 23:03 Adding missing object access hook invocations Mark Dilger <mark.dilger@enterprisedb.com>
2020-03-17 00:14 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-03-17 00:58 ` Mark Dilger <mark.dilger@enterprisedb.com>
2020-03-17 18:49 ` Andres Freund <andres@anarazel.de>
2020-03-17 19:39 ` Mark Dilger <mark.dilger@enterprisedb.com>
2020-03-18 04:33 ` Michael Paquier <michael@paquier.xyz>
2020-03-18 14:50 ` Mark Dilger <mark.dilger@enterprisedb.com>
2020-03-19 18:17 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-03-19 18:30 ` Mark Dilger <mark.dilger@enterprisedb.com>
2020-03-19 18:47 ` Mark Dilger <mark.dilger@enterprisedb.com>
2020-04-19 22:55 ` Michael Paquier <michael@paquier.xyz>
2020-04-19 23:43 ` Mark Dilger <mark.dilger@enterprisedb.com>
2020-04-20 17:32 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-04-21 01:39 ` Michael Paquier <michael@paquier.xyz>
2020-05-20 17:57 ` Robert Haas <robertmhaas@gmail.com>
2020-05-21 00:32 ` Michael Paquier <michael@paquier.xyz>
2020-05-23 05:21 ` Michael Paquier <michael@paquier.xyz>
2020-05-24 02:31 ` Alvaro Herrera <alvherre@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox