public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v9 1/2] Let ALTER TABLE exec routines deal with the relation
52+ messages / 5 participants
[nested] [flat]
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v9 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3349bcfaa7..bf7fd6e8ae 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4569,7 +4571,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4578,10 +4579,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4593,7 +4594,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4619,11 +4624,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5730,6 +5736,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--IS0zKkzwUGydFO0o
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v3 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 511f015a86..2784fb11f9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4393,7 +4395,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4402,10 +4403,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4417,7 +4418,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4443,11 +4448,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5545,6 +5551,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--J/dobhs11T7y2rNN
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v8 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ffb1308a0c..8e753f1efd 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4512,7 +4514,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4521,10 +4522,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4536,7 +4537,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4562,11 +4567,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5669,6 +5675,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--XsQoSWH+UP9D9v3l
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v6 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b2457a6924..b990063d38 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4513,7 +4515,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4522,10 +4523,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4537,7 +4538,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4563,11 +4568,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5670,6 +5676,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--LZvS9be/3tNcYl/X
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v8 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ffb1308a0c..8e753f1efd 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4512,7 +4514,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4521,10 +4522,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4536,7 +4537,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4562,11 +4567,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5669,6 +5675,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--XsQoSWH+UP9D9v3l
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 993da56d43..a8528a3423 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -156,6 +156,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -353,7 +355,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4405,7 +4407,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4414,10 +4415,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4429,7 +4430,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4455,11 +4460,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5562,6 +5568,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation
@ 2020-07-14 00:15 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Alvaro Herrera @ 2020-07-14 00:15 UTC (permalink / raw)
This means that ATExecCmd relies on AlteredRelationInfo->rel instead of
keeping the relation as a local variable; this is useful if the
subcommand needs to modify the relation internally. For example, a
subcommand that internally commits its transaction needs this.
---
src/backend/commands/tablecmds.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ac53f79ada..07f4f562ee 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,8 @@ typedef struct AlteredTableInfo
Oid relid; /* Relation to work on */
char relkind; /* Its relkind */
TupleDesc oldDesc; /* Pre-modification tuple descriptor */
+ /* Transiently set during Phase 2, normally set to NULL */
+ Relation rel;
/* Information saved by Phase 1 for Phase 2: */
List *subcmds[AT_NUM_PASSES]; /* Lists of AlterTableCmd */
/* Information saved by Phases 1/2 for Phase 3: */
@@ -354,7 +356,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context);
-static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+static void ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context);
static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab,
@@ -4326,7 +4328,6 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
List *subcmds = tab->subcmds[pass];
- Relation rel;
ListCell *lcmd;
if (subcmds == NIL)
@@ -4335,10 +4336,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
/*
* Appropriate lock was obtained by phase 1, needn't get it again
*/
- rel = relation_open(tab->relid, NoLock);
+ tab->rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
- ATExecCmd(wqueue, tab, rel,
+ ATExecCmd(wqueue, tab,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode, pass, context);
@@ -4350,7 +4351,11 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
- relation_close(rel, NoLock);
+ if (tab->rel)
+ {
+ relation_close(tab->rel, NoLock);
+ tab->rel = NULL;
+ }
}
}
@@ -4376,11 +4381,12 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* ATExecCmd: dispatch a subcommand to appropriate execution routine
*/
static void
-ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ATExecCmd(List **wqueue, AlteredTableInfo *tab,
AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass,
AlterTableUtilityContext *context)
{
ObjectAddress address = InvalidObjectAddress;
+ Relation rel = tab->rel;
switch (cmd->subtype)
{
@@ -5455,6 +5461,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
*/
tab = (AlteredTableInfo *) palloc0(sizeof(AlteredTableInfo));
tab->relid = relid;
+ tab->rel = NULL; /* set later */
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopyConstr(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
--
2.20.1
--ikeVEW9yuYc//A+q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-ALTER-TABLE-.-DETACH-CONCURRENTLY.patch"
^ permalink raw reply [nested|flat] 52+ messages in thread
* running logical replication as the subscription owner
@ 2023-03-03 16:02 Robert Haas <[email protected]>
2023-03-24 07:59 ` Re: running logical replication as the subscription owner Jeff Davis <[email protected]>
0 siblings, 1 reply; 52+ messages in thread
From: Robert Haas @ 2023-03-03 16:02 UTC (permalink / raw)
To: pgsql-hackers
Hi,
Here's a patch against master for $SUBJECT. It lacks documentation
changes and might have bugs, so please review if you're concerned
about this issue.
To recap, under CVE-2020-14349, Noah documented that untrusted users
shouldn't own tables into which the system is performing logical
replication. Otherwise, the users could hook up triggers or default
expressions or whatever to those tables and they would execute with
the subscription owner's privileges, which would allow the table
owners to escalate to superuser. However, I'm unsatisfied with just
documenting the hazard, because I feel like almost everyone who uses
logical replication wants to do the exact thing that this
documentation says you shouldn't do. If you don't use logical
replication or run everything as the superuser or just don't care
about security, well then you don't have any problem here, but
otherwise you probably do.
The proposed fix is to perform logical replication actions (SELECT,
INSERT, UPDATE, DELETE, and TRUNCATE) as the user who owns the table
rather than as the owner of the subscription. The session still runs
as the subscription owner, but the active user ID is switched to the
table owner for the duration of each operation. To prevent table
owners from doing tricky things to attack the subscription owner, we
impose SECURITY_RESTRICTED_OPERATION while running as the table owner.
To avoid inconveniencing users when this restriction adds no
meaningful security, we refrain from imposing
SECURITY_RESTRICTED_OPERATION when the table owner can SET ROLE to the
subscription owner anyway. Such a user need not use logical
replication to break into the subscription owner's account: they have
access to it anyway.
There is also a possibility of an attack in the other direction. Maybe
the subscription owner would like to obtain the table owner's
permissions, or at the very least, use logical replication as a
vehicle to perform operations they can't perform directly. A malicious
subscription owner could hook up logical replication to a table into
which the table owner doesn't want replication to occur. To block such
attacks, the patch requires that the subscription owner have the
ability to SET ROLE to each table owner. If the subscription owner is
a superuser, which is usual, this will be automatic. Otherwise, the
superuser will need to grant to the subscription owner the roles that
own relevant tables. This can usefully serve as a kind of access
control to make sure that the subscription doesn't touch any tables
other than the ones it's supposed to be touching: just make those
tables owned by a different user and don't grant them to the
subscription owner. Previously, we provided no way at all of
controlling the tables that replication can target.
This fix interacts in an interesting way with Mark Dilger's work,
committed by Jeff Davis, to make logical replication respect table
permissions. I initially thought that with this change, that commit
would end up just being reverted, with the permissions scheme
described above replacing the existing one. However, I then realized
that it's still good to perform those checks. Normally, a table owner
can do any DML operation on a table they own, so those checks will
never fail. However, if a table owner has revoked their ability to,
say, INSERT into one of their own tables, then logical replication
shouldn't bypass that and perform the INSERT anyway. So I now think
that the checks added by that commit complement the ones added by this
proposed patch, rather than competing with them.
It is unclear to me whether we should try to back-port this. It's
definitely a behavior change, and changing the behavior in the back
branches is not a nice thing to do. On the other hand, at least in my
opinion, the security consequences of the status quo are pretty dire.
I tend to suspect that a really high percentage of people who are
using logical replication at all are vulnerable to this, and lots of
people having a way to escalate to superuser isn't good.
Comments?
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
[application/octet-stream] v1-0001-Perform-logical-replication-actions-as-the-table-.patch (21.4K, ../../CA+TgmoaSCkg9ww9oppPqqs+9RVqCexYCE6Aq=UsYPfnOoDeFkw@mail.gmail.com/2-v1-0001-Perform-logical-replication-actions-as-the-table-.patch)
download | inline diff:
From 9168aa37141a59efccf3cea419ff4cacff7c1bb0 Mon Sep 17 00:00:00 2001
From: Robert Haas <[email protected]>
Date: Wed, 1 Mar 2023 12:59:02 -0500
Subject: [PATCH v1] Perform logical replication actions as the table owner.
Up until now, logical replication actions have been performed as the
subscription owner, who will generally be a superuser. Commit
cec57b1a0fbcd3833086ba686897c5883e0a2afc documented hazards
associated with that situation, namely, that any user who owns a
table on the subscriber side could assume the privileges of the
subscription owner by attaching a trigger, expression index, or
some other kind of executable code to it. As a remedy, it suggested
not creating configurations where users who are not fully trusted
own tables on the subscriber.
Although that will work, it basically precludes using logical
replication in the way that people typically want to use it,
namely, to replicate a database from one node to another
without necessarily having any restrictions on which database
users can own tables. So, instead, change logical replication to
execute INSERT, UPDATE, DELETE, and TRUNCATE operations as the
table owner when they are replicated.
Since this involves switching the active user frequently within
a session that is authenticated as the subscription user, also
impose SECURITY_RESTRICTED_OPEATION restrictions on logical
replication code. As an exception, if the table owner can SET
ROLE to the subscription owner, these restrictions have no
security value, so don't impose them in that case.
Subscription owners are now required to have the ability to
SET ROLE to every role that owns a table that the subscription
is replicating. If they don't, replication will fail. Superusers,
who normally own subscriptions, satisfy this property by default.
Non-superusers users who own subscriptions will needed to be
granted the roles that own relevant tables.
---
src/backend/commands/tablecmds.c | 20 ++-
src/backend/replication/logical/worker.c | 22 ++-
src/backend/utils/init/Makefile | 3 +-
src/backend/utils/init/meson.build | 3 +-
src/backend/utils/init/usercontext.c | 92 ++++++++++++
src/include/commands/tablecmds.h | 3 +-
src/include/utils/usercontext.h | 26 ++++
src/test/subscription/t/027_nosuperuser.pl | 165 ++++++++++-----------
8 files changed, 241 insertions(+), 93 deletions(-)
create mode 100644 src/backend/utils/init/usercontext.c
create mode 100644 src/include/utils/usercontext.h
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 62d9917ca3..a53ab38eb3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -103,6 +103,7 @@
#include "utils/syscache.h"
#include "utils/timestamp.h"
#include "utils/typcache.h"
+#include "utils/usercontext.h"
/*
* ON COMMIT action list
@@ -1761,7 +1762,7 @@ ExecuteTruncate(TruncateStmt *stmt)
}
ExecuteTruncateGuts(rels, relids, relids_logged,
- stmt->behavior, stmt->restart_seqs);
+ stmt->behavior, stmt->restart_seqs, false);
/* And close the rels */
foreach(cell, rels)
@@ -1789,7 +1790,8 @@ void
ExecuteTruncateGuts(List *explicit_rels,
List *relids,
List *relids_logged,
- DropBehavior behavior, bool restart_seqs)
+ DropBehavior behavior, bool restart_seqs,
+ bool run_as_table_owner)
{
List *rels;
List *seq_relids = NIL;
@@ -1928,7 +1930,14 @@ ExecuteTruncateGuts(List *explicit_rels,
resultRelInfo = resultRelInfos;
foreach(cell, rels)
{
+ UserContext ucxt;
+
+ if (run_as_table_owner)
+ SwitchToUntrustedUser(resultRelInfo->ri_RelationDesc->rd_rel->relowner,
+ &ucxt);
ExecBSTruncateTriggers(estate, resultRelInfo);
+ if (run_as_table_owner)
+ RestoreUserContext(&ucxt);
resultRelInfo++;
}
@@ -2133,7 +2142,14 @@ ExecuteTruncateGuts(List *explicit_rels,
resultRelInfo = resultRelInfos;
foreach(cell, rels)
{
+ UserContext ucxt;
+
+ if (run_as_table_owner)
+ SwitchToUntrustedUser(resultRelInfo->ri_RelationDesc->rd_rel->relowner,
+ &ucxt);
ExecASTruncateTriggers(estate, resultRelInfo);
+ if (run_as_table_owner)
+ RestoreUserContext(&ucxt);
resultRelInfo++;
}
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index cfb2ab6248..9e07fbc6a8 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -207,6 +207,7 @@
#include "utils/rls.h"
#include "utils/syscache.h"
#include "utils/timeout.h"
+#include "utils/usercontext.h"
#define NAPTIME_PER_CYCLE 1000 /* max sleep time between cycles (1s) */
@@ -2408,6 +2409,7 @@ apply_handle_insert(StringInfo s)
LogicalRepRelMapEntry *rel;
LogicalRepTupleData newtup;
LogicalRepRelId relid;
+ UserContext ucxt;
ApplyExecutionData *edata;
EState *estate;
TupleTableSlot *remoteslot;
@@ -2436,6 +2438,9 @@ apply_handle_insert(StringInfo s)
return;
}
+ /* Make sure that any user-supplied code runs as the table owner. */
+ SwitchToUntrustedUser(rel->localrel->rd_rel->relowner, &ucxt);
+
/* Set relation for error callback */
apply_error_callback_arg.rel = rel;
@@ -2465,6 +2470,8 @@ apply_handle_insert(StringInfo s)
/* Reset relation for error callback */
apply_error_callback_arg.rel = NULL;
+ RestoreUserContext(&ucxt);
+
logicalrep_rel_close(rel, NoLock);
end_replication_step();
@@ -2543,6 +2550,7 @@ apply_handle_update(StringInfo s)
{
LogicalRepRelMapEntry *rel;
LogicalRepRelId relid;
+ UserContext ucxt;
ApplyExecutionData *edata;
EState *estate;
LogicalRepTupleData oldtup;
@@ -2582,6 +2590,9 @@ apply_handle_update(StringInfo s)
/* Check if we can do the update. */
check_relation_updatable(rel);
+ /* Make sure that any user-supplied code runs as the table owner. */
+ SwitchToUntrustedUser(rel->localrel->rd_rel->relowner, &ucxt);
+
/* Initialize the executor state. */
edata = create_edata_for_relation(rel);
estate = edata->estate;
@@ -2632,6 +2643,8 @@ apply_handle_update(StringInfo s)
/* Reset relation for error callback */
apply_error_callback_arg.rel = NULL;
+ RestoreUserContext(&ucxt);
+
logicalrep_rel_close(rel, NoLock);
end_replication_step();
@@ -2713,6 +2726,7 @@ apply_handle_delete(StringInfo s)
LogicalRepRelMapEntry *rel;
LogicalRepTupleData oldtup;
LogicalRepRelId relid;
+ UserContext ucxt;
ApplyExecutionData *edata;
EState *estate;
TupleTableSlot *remoteslot;
@@ -2747,6 +2761,9 @@ apply_handle_delete(StringInfo s)
/* Check if we can do the delete. */
check_relation_updatable(rel);
+ /* Make sure that any user-supplied code runs as the table owner. */
+ SwitchToUntrustedUser(rel->localrel->rd_rel->relowner, &ucxt);
+
/* Initialize the executor state. */
edata = create_edata_for_relation(rel);
estate = edata->estate;
@@ -2772,6 +2789,8 @@ apply_handle_delete(StringInfo s)
/* Reset relation for error callback */
apply_error_callback_arg.rel = NULL;
+ RestoreUserContext(&ucxt);
+
logicalrep_rel_close(rel, NoLock);
end_replication_step();
@@ -3219,7 +3238,8 @@ apply_handle_truncate(StringInfo s)
relids,
relids_logged,
DROP_RESTRICT,
- restart_seqs);
+ restart_seqs,
+ true);
foreach(lc, remote_rels)
{
LogicalRepRelMapEntry *rel = lfirst(lc);
diff --git a/src/backend/utils/init/Makefile b/src/backend/utils/init/Makefile
index 362569393b..18c947464f 100644
--- a/src/backend/utils/init/Makefile
+++ b/src/backend/utils/init/Makefile
@@ -15,6 +15,7 @@ include $(top_builddir)/src/Makefile.global
OBJS = \
globals.o \
miscinit.o \
- postinit.o
+ postinit.o \
+ usercontext.o
include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/utils/init/meson.build b/src/backend/utils/init/meson.build
index b5ab154055..186be1381d 100644
--- a/src/backend/utils/init/meson.build
+++ b/src/backend/utils/init/meson.build
@@ -3,4 +3,5 @@
backend_sources += files(
'globals.c',
'miscinit.c',
- 'postinit.c')
+ 'postinit.c',
+ 'usercontext.c')
diff --git a/src/backend/utils/init/usercontext.c b/src/backend/utils/init/usercontext.c
new file mode 100644
index 0000000000..88a7e55478
--- /dev/null
+++ b/src/backend/utils/init/usercontext.c
@@ -0,0 +1,92 @@
+/*-------------------------------------------------------------------------
+ *
+ * usercontext.c
+ * Convenience functions for running code as a different database user.
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ * src/backend/utils/init/usercontext.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "miscadmin.h"
+#include "utils/acl.h"
+#include "utils/guc.h"
+#include "utils/usercontext.h"
+
+/*
+ * Temporarily switch to a new user ID.
+ *
+ * If the current user doesn't have permission to SET ROLE to the new user,
+ * an ERROR occurs.
+ *
+ * If the new user doesn't have permission to SET ROLE to the current user,
+ * SECURITY_RESTRICTED_OPERATION is imposed and a new GUC nest level is
+ * created so that any settings changes can be rolled back.
+ */
+void
+SwitchToUntrustedUser(Oid userid, UserContext *context)
+{
+ /* Get the current user ID and security context. */
+ GetUserIdAndSecContext(&context->save_userid,
+ &context->save_sec_context);
+
+ /* Check that we have sufficient privileges to assume the target role. */
+ if (!member_can_set_role(context->save_userid, userid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("role \"%s\" cannot SET ROLE to \"%s\"",
+ GetUserNameFromId(context->save_userid, false),
+ GetUserNameFromId(userid, false))));
+
+ /*
+ * Try to prevent the user to which we're switching from assuming the
+ * privileges of the current user, unless they can SET ROLE to that user
+ * anyway.
+ */
+ if (member_can_set_role(userid, context->save_userid))
+ {
+ /*
+ * Each user can SET ROLE to the other, so there's no point in
+ * imposing any security restrictions. Just let the user do whatever
+ * they want.
+ */
+ SetUserIdAndSecContext(userid, context->save_sec_context);
+ context->save_nestlevel = -1;
+ }
+ else
+ {
+ int sec_context = context->save_sec_context;
+
+ /*
+ * This user can SET ROLE to the target user, but not the other way
+ * around, so protect ourselves against the target user by setting
+ * SECURITY_RESTRICTED_OPERATION to prevent certain changes to the
+ * session state. Also set up a new GUC nest level, so that we can roll
+ * back any GUC changes that may be made by code running as the target
+ * user, inasmuch as they could be malicious.
+ */
+ sec_context |= SECURITY_RESTRICTED_OPERATION;
+ SetUserIdAndSecContext(userid, sec_context);
+ context->save_nestlevel = NewGUCNestLevel();
+ }
+}
+
+/*
+ * Switch back to the original user ID.
+ *
+ * If we created a new GUC nest level, also role back any changes that were
+ * made within it.
+ */
+void
+RestoreUserContext(UserContext *context)
+{
+ if (context->save_nestlevel != -1)
+ AtEOXact_GUC(false, context->save_nestlevel);
+ SetUserIdAndSecContext(context->save_userid, context->save_sec_context);
+}
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index e7c2b91a58..17b9404937 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -60,7 +60,8 @@ extern void ExecuteTruncateGuts(List *explicit_rels,
List *relids,
List *relids_logged,
DropBehavior behavior,
- bool restart_seqs);
+ bool restart_seqs,
+ bool run_as_table_owner);
extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass);
diff --git a/src/include/utils/usercontext.h b/src/include/utils/usercontext.h
new file mode 100644
index 0000000000..a8195c194d
--- /dev/null
+++ b/src/include/utils/usercontext.h
@@ -0,0 +1,26 @@
+/*-------------------------------------------------------------------------
+ *
+ * usercontext.h
+ * Convenience functions for running code as a different database user.
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef USERCONTEXT_H
+#define USERCONTEXT_H
+
+/*
+ * When temporarily changing to run as a different user, this structure
+ * holds the details needed to restore the original state.
+ */
+typedef struct UserContext
+{
+ Oid save_userid;
+ int save_sec_context;
+ int save_nestlevel;
+} UserContext;
+
+/* Function prototypes. */
+extern void SwitchToUntrustedUser(Oid userid, UserContext *context);
+extern void RestoreUserContext(UserContext *context);
+
+#endif /* USERCONTEXT_H */
diff --git a/src/test/subscription/t/027_nosuperuser.pl b/src/test/subscription/t/027_nosuperuser.pl
index 59192dbe2f..44d708877b 100644
--- a/src/test/subscription/t/027_nosuperuser.pl
+++ b/src/test/subscription/t/027_nosuperuser.pl
@@ -76,22 +76,6 @@ sub grant_superuser
ALTER ROLE $role SUPERUSER));
}
-sub revoke_bypassrls
-{
- my ($role) = @_;
- $node_subscriber->safe_psql(
- 'postgres', qq(
- ALTER ROLE $role NOBYPASSRLS));
-}
-
-sub grant_bypassrls
-{
- my ($role) = @_;
- $node_subscriber->safe_psql(
- 'postgres', qq(
- ALTER ROLE $role BYPASSRLS));
-}
-
# Create publisher and subscriber nodes with schemas owned and published by
# "regress_alice" but subscribed and replicated by different role
# "regress_admin". For partitioned tables, layout the partitions differently
@@ -177,59 +161,32 @@ expect_failure(
2,
5,
7,
- qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
+ qr/ERROR: ( [A-Z0-9]+:)? role "regress_admin" cannot SET ROLE to "regress_alice"/msi,
"non-superuser admin fails to replicate update");
grant_superuser("regress_admin");
expect_replication("alice.unpartitioned", 2, 7, 9,
"admin with restored superuser privilege replicates update");
-# Grant INSERT, UPDATE, DELETE privileges on the target tables to
-# "regress_admin" so that superuser privileges are not necessary for
-# replication.
-#
-# Note that UPDATE and DELETE also require SELECT privileges, which
-# will be granted in subsequent test.
-#
+# Privileges on the target role suffice for non-superuser replication.
$node_subscriber->safe_psql(
'postgres', qq(
ALTER ROLE regress_admin NOSUPERUSER;
-SET SESSION AUTHORIZATION regress_alice;
-GRANT INSERT,UPDATE,DELETE ON
- alice.unpartitioned,
- alice.hashpart, alice.hashpart_a, alice.hashpart_b
- TO regress_admin;
-REVOKE SELECT ON alice.unpartitioned FROM regress_admin;
+GRANT regress_alice TO regress_admin;
));
publish_insert("alice.unpartitioned", 11);
expect_replication("alice.unpartitioned", 3, 7, 11,
- "nosuperuser admin with INSERT privileges can replicate into unpartitioned"
+ "nosuperuser admin with privileges on role can replicate INSERT into unpartitioned"
);
publish_update("alice.unpartitioned", 7 => 13);
-expect_failure(
- "alice.unpartitioned",
- 3,
- 7,
- 11,
- qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
- "non-superuser admin without SELECT privileges fails to replicate update"
+expect_replication("alice.unpartitioned", 3, 9, 13,
+ "nosuperuser admin with privileges on role can replicate UPDATE into unpartitioned"
);
-# Now grant SELECT
-#
-$node_subscriber->safe_psql(
- 'postgres', qq(
-SET SESSION AUTHORIZATION regress_alice;
-GRANT SELECT ON
- alice.unpartitioned,
- alice.hashpart, alice.hashpart_a, alice.hashpart_b
- TO regress_admin;
-));
-
publish_delete("alice.unpartitioned", 9);
expect_replication("alice.unpartitioned", 2, 11, 13,
- "nosuperuser admin with all table privileges can replicate into unpartitioned"
+ "nosuperuser admin with privileges on role can replicate DELETE into unpartitioned"
);
# Test partitioning
@@ -240,80 +197,114 @@ publish_insert("alice.hashpart", 103);
publish_update("alice.hashpart", 102 => 120);
publish_delete("alice.hashpart", 101);
expect_replication("alice.hashpart", 2, 103, 120,
- "nosuperuser admin with all table privileges can replicate into hashpart"
+ "nosuperuser admin with privileges on role can replicate into hashpart"
);
-
-# Enable RLS on the target table and check that "regress_admin" can
-# only replicate into it when superuser or bypassrls.
-#
+# Force RLS on the target table and check that replication fails.
$node_subscriber->safe_psql(
'postgres', qq(
SET SESSION AUTHORIZATION regress_alice;
ALTER TABLE alice.unpartitioned ENABLE ROW LEVEL SECURITY;
+ALTER TABLE alice.unpartitioned FORCE ROW LEVEL SECURITY;
));
-revoke_superuser("regress_admin");
publish_insert("alice.unpartitioned", 15);
expect_failure(
"alice.unpartitioned",
2,
11,
13,
- qr/ERROR: ( [A-Z0-9]+:)? user "regress_admin" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
- "non-superuser admin fails to replicate insert into rls enabled table");
-grant_superuser("regress_admin");
+ qr/ERROR: ( [A-Z0-9]+:)? user "regress_alice" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
+ "replication of insert into table with forced rls fails");
+
+# Since replication acts as the table owner, replication will succeed if we don't force it.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+ALTER TABLE alice.unpartitioned NO FORCE ROW LEVEL SECURITY;
+));
expect_replication("alice.unpartitioned", 3, 11, 15,
- "admin with restored superuser privilege replicates insert into rls enabled unpartitioned"
+ "non-superuser admin can replicate insert if rls is not forced"
);
-revoke_superuser("regress_admin");
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+ALTER TABLE alice.unpartitioned FORCE ROW LEVEL SECURITY;
+));
publish_update("alice.unpartitioned", 11 => 17);
expect_failure(
"alice.unpartitioned",
3,
11,
15,
- qr/ERROR: ( [A-Z0-9]+:)? user "regress_admin" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
- "non-superuser admin fails to replicate update into rls enabled unpartitioned"
+ qr/ERROR: ( [A-Z0-9]+:)? user "regress_alice" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
+ "replication of update into table with forced rls fails"
);
-
-grant_bypassrls("regress_admin");
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+ALTER TABLE alice.unpartitioned NO FORCE ROW LEVEL SECURITY;
+));
expect_replication("alice.unpartitioned", 3, 13, 17,
- "admin with bypassrls replicates update into rls enabled unpartitioned");
+ "non-superuser admin can replicate update if rls is not forced");
-revoke_bypassrls("regress_admin");
-publish_delete("alice.unpartitioned", 13);
+# Remove some of alice's privileges on her own table. Then replication should fail.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+REVOKE SELECT, INSERT ON alice.unpartitioned FROM regress_alice;
+));
+publish_insert("alice.unpartitioned", 19);
expect_failure(
"alice.unpartitioned",
3,
13,
17,
- qr/ERROR: ( [A-Z0-9]+:)? user "regress_admin" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
- "non-superuser admin without bypassrls fails to replicate delete into rls enabled unpartitioned"
+ qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
+ "replication of insert fails if table owner lacks insert permission"
);
-grant_bypassrls("regress_admin");
-expect_replication("alice.unpartitioned", 2, 15, 17,
- "admin with bypassrls replicates delete into rls enabled unpartitioned");
-grant_superuser("regress_admin");
-# Alter the subscription owner to "regress_alice". She has neither superuser
-# nor bypassrls, but as the table owner should be able to replicate.
-#
+# alice needs INSERT but not SELECT to replicate an INSERT.
$node_subscriber->safe_psql(
'postgres', qq(
-ALTER SUBSCRIPTION admin_sub DISABLE;
-ALTER ROLE regress_alice SUPERUSER;
-ALTER SUBSCRIPTION admin_sub OWNER TO regress_alice;
-ALTER ROLE regress_alice NOSUPERUSER;
-ALTER SUBSCRIPTION admin_sub ENABLE;
+GRANT INSERT ON alice.unpartitioned TO regress_alice;
));
+expect_replication("alice.unpartitioned", 4, 13, 19,
+ "restoring insert permission permits replication to continue");
-publish_insert("alice.unpartitioned", 23);
-publish_update("alice.unpartitioned", 15 => 25);
-publish_delete("alice.unpartitioned", 17);
-expect_replication("alice.unpartitioned", 2, 23, 25,
- "nosuperuser nobypassrls table owner can replicate delete into unpartitioned despite rls"
+# Now let's try an UPDATE and a DELETE.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+REVOKE UPDATE, DELETE ON alice.unpartitioned FROM regress_alice;
+));
+publish_update("alice.unpartitioned", 13 => 21);
+publish_delete("alice.unpartitioned", 15);
+expect_failure(
+ "alice.unpartitioned",
+ 4,
+ 13,
+ 19,
+ qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
+ "replication of update/delete fails if table owner lacks corresponding permission"
);
+# Restoring UPDATE and DELETE is insufficient.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+GRANT UPDATE, DELETE ON alice.unpartitioned TO regress_alice;
+));
+expect_failure(
+ "alice.unpartitioned",
+ 4,
+ 13,
+ 19,
+ qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
+ "replication of update/delete fails if table owner lacks SELECT permission"
+);
+
+# alice needs INSERT but not SELECT to replicate an INSERT.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+GRANT SELECT ON alice.unpartitioned TO regress_alice;
+));
+expect_replication("alice.unpartitioned", 3, 17, 21,
+ "restoring SELECT permission permits replication to continue");
+
done_testing();
--
2.37.1 (Apple Git-137.1)
^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: running logical replication as the subscription owner
2023-03-03 16:02 running logical replication as the subscription owner Robert Haas <[email protected]>
@ 2023-03-24 07:59 ` Jeff Davis <[email protected]>
2023-03-24 14:00 ` Re: running logical replication as the subscription owner Robert Haas <[email protected]>
0 siblings, 1 reply; 52+ messages in thread
From: Jeff Davis @ 2023-03-24 07:59 UTC (permalink / raw)
To: Robert Haas <[email protected]>; pgsql-hackers
On Fri, 2023-03-03 at 11:02 -0500, Robert Haas wrote:
> Hi,
>
> Here's a patch against master for $SUBJECT. It lacks documentation
> changes and might have bugs, so please review if you're concerned
> about this issue.
I think the subject has a typo, you mean "as the table owner", right?
> However, I'm unsatisfied with just
> documenting the hazard, because I feel like almost everyone who uses
> logical replication wants to do the exact thing that this
> documentation says you shouldn't do.
+1
> The proposed fix is to perform logical replication actions (SELECT,
> INSERT, UPDATE, DELETE, and TRUNCATE) as the user who owns the table
> rather than as the owner of the subscription. The session still runs
> as the subscription owner, but the active user ID is switched to the
> table owner for the duration of each operation. To prevent table
> owners from doing tricky things to attack the subscription owner, we
> impose SECURITY_RESTRICTED_OPERATION while running as the table
> owner.
+1
> To avoid inconveniencing users when this restriction adds no
> meaningful security, we refrain from imposing
> SECURITY_RESTRICTED_OPERATION when the table owner can SET ROLE to
> the
> subscription owner anyway.
That's a little confusing, why not just always use the
SECURITY_RESTRICTED_OPERATION? Is there a use case I'm missing?
> There is also a possibility of an attack in the other direction.
> Maybe
> the subscription owner would like to obtain the table owner's
> permissions, or at the very least, use logical replication as a
> vehicle to perform operations they can't perform directly. A
> malicious
> subscription owner could hook up logical replication to a table into
> which the table owner doesn't want replication to occur. To block
> such
> attacks, the patch requires that the subscription owner have the
> ability to SET ROLE to each table owner.
It would be really nice if this could be done with some kind of
ordinary privilege rather than requiring SET ROLE. A user might expect
that INSERT/UPDATE/DELETE/TRUNCATE privileges are enough. Or
pg_write_all_data.
I can see theoretically that a table owner might write some dangerous
code and attach it to their table. But I don't quite understand why
they would do that. If the code was vulnerable to attack, would that
mean that they couldn't even insert into their own table safely?
Requiring SET ROLE seems like it makes the subscription role into
something very close to a superuser. And that takes away some of the
benefits of delegating to non-superusers.
> If the subscription owner is
> a superuser, which is usual, this will be automatic. Otherwise, the
> superuser will need to grant to the subscription owner the roles that
> own relevant tables. This can usefully serve as a kind of access
> control to make sure that the subscription doesn't touch any tables
> other than the ones it's supposed to be touching: just make those
> tables owned by a different user and don't grant them to the
> subscription owner. Previously, we provided no way at all of
> controlling the tables that replication can target.
We check for ordinary access privileges, which I think is your next
point, so the above paragraph confuses me a bit.
> This fix interacts in an interesting way with Mark Dilger's work,
> committed by Jeff Davis, to make logical replication respect table
> permissions. I initially thought that with this change, that commit
> would end up just being reverted, with the permissions scheme
> described above replacing the existing one. However, I then realized
> that it's still good to perform those checks. Normally, a table owner
> can do any DML operation on a table they own, so those checks will
> never fail. However, if a table owner has revoked their ability to,
> say, INSERT into one of their own tables, then logical replication
> shouldn't bypass that and perform the INSERT anyway. So I now think
> that the checks added by that commit complement the ones added by
> this
> proposed patch, rather than competing with them.
That's an interesting case.
> It is unclear to me whether we should try to back-port this. It's
> definitely a behavior change, and changing the behavior in the back
> branches is not a nice thing to do. On the other hand, at least in my
> opinion, the security consequences of the status quo are pretty dire.
> I tend to suspect that a really high percentage of people who are
> using logical replication at all are vulnerable to this, and lots of
> people having a way to escalate to superuser isn't good.
It's worth considering given that most subscription owners are
superusers anyway. What plausible cases might it break?
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: running logical replication as the subscription owner
2023-03-03 16:02 running logical replication as the subscription owner Robert Haas <[email protected]>
2023-03-24 07:59 ` Re: running logical replication as the subscription owner Jeff Davis <[email protected]>
@ 2023-03-24 14:00 ` Robert Haas <[email protected]>
2023-03-24 16:58 ` Re: running logical replication as the subscription owner Mark Dilger <[email protected]>
0 siblings, 1 reply; 52+ messages in thread
From: Robert Haas @ 2023-03-24 14:00 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: pgsql-hackers; Andres Freund <[email protected]>; Noah Misch <[email protected]>
On Fri, Mar 24, 2023 at 3:59 AM Jeff Davis <[email protected]> wrote:
> That's a little confusing, why not just always use the
> SECURITY_RESTRICTED_OPERATION? Is there a use case I'm missing?
Some concern was expressed -- not sure exactly where the email is
exactly, and it might've been on pgsql-security -- that doing that
categorically might break things that are currently working. The
people who were concerned included Andres and I forget who else. My
gut reaction was the same as yours, just do it always and don't worry
about it. But if people think that users are likely to run afoul of
the SECURITY_RESTRICTED_OPERATION restrictions in practice, then this
is better, and the implementation complexity isn't high. We could even
think of extending this kind of logic to other places where
SECURITY_RESTRICTED_OPERATION is enforced, if desired.
> It would be really nice if this could be done with some kind of
> ordinary privilege rather than requiring SET ROLE. A user might expect
> that INSERT/UPDATE/DELETE/TRUNCATE privileges are enough. Or
> pg_write_all_data.
>
> I can see theoretically that a table owner might write some dangerous
> code and attach it to their table. But I don't quite understand why
> they would do that. If the code was vulnerable to attack, would that
> mean that they couldn't even insert into their own table safely?
>
> Requiring SET ROLE seems like it makes the subscription role into
> something very close to a superuser. And that takes away some of the
> benefits of delegating to non-superusers.
I am not thrilled with this either, but if you can arrange to run code
as a certain user without the ability to SET ROLE to that user then
there is, by definition, a potential privilege escalation. I don't
think we can just dismiss that as a non-issue. If the ability of one
user to potentially become some other user weren't a problem, we
wouldn't need any patch at all. Imagine for example that the table
owner has a trigger which doesn't sanitize search_path. The
subscription owner can potentially leverage that to get the table
owner's privileges.
More generally, Stephen Frost has elsewhere argued that we should want
the subscription owner to be a very low-privilege user, so that if
their privileges get stolen, it's no big deal. I disagree with that. I
think it's always a problem if one user can get unauthorized access to
another user's account, regardless of exactly what those accounts can
do. I think our goal should be to make it safe for the subscription
owner to be a very high-privilege user, because you're going to need
to be a very high-privilege user to set up replication. And if you do
have that level of privilege, it's more convenient and simpler if you
can just own the subscription yourself, rather than having to make a
dummy account to own it. To put that another way, I think that what
people are going to want to do in a lot of cases is have the superuser
own the subscription, so I think we need to make that case safe,
whatever it takes. In cases where the subscription owner isn't the
superuser, I think the next most likely possibility is that the
subscription owner is some kind of almost-super-user, like a
CREATEROLE user or someone running with rds_superuser or similar on
some PG fork. So that needs to be safe too, and I think this does
that. Having the subscription owner be some random user that doesn't
have a lot of privileges doesn't seem particularly useful to me. If it
were unproblematic to allow that, sure, but considering how easy it
would be for that low-privilege user to steal table owner privileges,
I don't think it makes sense.
> > It is unclear to me whether we should try to back-port this. It's
> > definitely a behavior change, and changing the behavior in the back
> > branches is not a nice thing to do. On the other hand, at least in my
> > opinion, the security consequences of the status quo are pretty dire.
> > I tend to suspect that a really high percentage of people who are
> > using logical replication at all are vulnerable to this, and lots of
> > people having a way to escalate to superuser isn't good.
>
> It's worth considering given that most subscription owners are
> superusers anyway. What plausible cases might it break?
AFAIU, the main concern is about the SECURITY_RESTRICTED_OPERATION
flag interacting badly with things people are already doing. Other
problems seem possible, e.g. if you're doing something that gets the
current user name and does something with it, the answer's going to
change, and you might like the new answer more or less than the old
one. It's a little hard to predict who will be inconvenienced in what
ways when you change behavior, but problems are certainly possible.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: running logical replication as the subscription owner
2023-03-03 16:02 running logical replication as the subscription owner Robert Haas <[email protected]>
2023-03-24 07:59 ` Re: running logical replication as the subscription owner Jeff Davis <[email protected]>
2023-03-24 14:00 ` Re: running logical replication as the subscription owner Robert Haas <[email protected]>
@ 2023-03-24 16:58 ` Mark Dilger <[email protected]>
2023-03-24 18:35 ` Re: running logical replication as the subscription owner Robert Haas <[email protected]>
0 siblings, 1 reply; 52+ messages in thread
From: Mark Dilger @ 2023-03-24 16:58 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Jeff Davis <[email protected]>; pgsql-hackers; Andres Freund <[email protected]>; Noah Misch <[email protected]>
> On Mar 24, 2023, at 7:00 AM, Robert Haas <[email protected]> wrote:
>
> More generally, Stephen Frost has elsewhere argued that we should want
> the subscription owner to be a very low-privilege user, so that if
> their privileges get stolen, it's no big deal. I disagree with that. I
> think it's always a problem if one user can get unauthorized access to
> another user's account, regardless of exactly what those accounts can
> do. I think our goal should be to make it safe for the subscription
> owner to be a very high-privilege user, because you're going to need
> to be a very high-privilege user to set up replication. And if you do
> have that level of privilege, it's more convenient and simpler if you
> can just own the subscription yourself, rather than having to make a
> dummy account to own it. To put that another way, I think that what
> people are going to want to do in a lot of cases is have the superuser
> own the subscription, so I think we need to make that case safe,
> whatever it takes.
I also think the subscription owner should be a low-privileged user, owing to the risk of the publisher injecting malicious content into the publication. I think you are focused on all the bad actors on the subscription-side database and what they can do to each other. That's also valid, but I get the impression that you're losing sight of the risk posed by malicious publishers. Or maybe you aren't, and can explain?
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: running logical replication as the subscription owner
2023-03-03 16:02 running logical replication as the subscription owner Robert Haas <[email protected]>
2023-03-24 07:59 ` Re: running logical replication as the subscription owner Jeff Davis <[email protected]>
2023-03-24 14:00 ` Re: running logical replication as the subscription owner Robert Haas <[email protected]>
2023-03-24 16:58 ` Re: running logical replication as the subscription owner Mark Dilger <[email protected]>
@ 2023-03-24 18:35 ` Robert Haas <[email protected]>
2023-03-25 09:24 ` Re: running logical replication as the subscription owner Jelte Fennema <[email protected]>
0 siblings, 1 reply; 52+ messages in thread
From: Robert Haas @ 2023-03-24 18:35 UTC (permalink / raw)
To: Mark Dilger <[email protected]>; +Cc: Jeff Davis <[email protected]>; pgsql-hackers; Andres Freund <[email protected]>; Noah Misch <[email protected]>
On Fri, Mar 24, 2023 at 12:58 PM Mark Dilger
<[email protected]> wrote:
> I also think the subscription owner should be a low-privileged user, owing to the risk of the publisher injecting malicious content into the publication. I think you are focused on all the bad actors on the subscription-side database and what they can do to each other. That's also valid, but I get the impression that you're losing sight of the risk posed by malicious publishers. Or maybe you aren't, and can explain?
You have a point.
As things stand today, if you think somebody's going to send you
changes to tables other than the ones you intend to replicate, you
could handle that by making sure that the user that owns the
subscription only has permission to write to the tables that are
expected to receive replicated data. It's a bit awkward to set up
because you have to initially make the subscription owner a superuser
and then later remove the superuser bit, so I think this is another
argument for the pg_create_subscription patch posted elsewhere, but if
you're a superuser yourself, you can do it. However, with this patch,
that wouldn't work any more, because the permissions checks don't
happen until after we've switched to the target role. You could
alternatively set up a user to own the subscription who has the
ability to SET ROLE to some users and not others, but that only lets
you restrict replication based on which user owns the tables, rather
than which specific tables are getting data replicated into them. That
actually wouldn't work today, and with the patch it would start
working, so basically the effect of the patch on the problem that you
mention would be to remove the ability to filter by specific table and
add the ability to filter by owning role.
I don't know how bad that sounds to you, and if it does sound bad, I
don't immediately see how to mitigate it. As I said to Jeff, if you
can replicate into a table that has a casually-written SECURITY
INVOKER trigger on it, you can probably hack into the table owner's
account. So I think that if we allow user A to replicate into user B's
table with fewer privileges than A-can-set-role-to-B, we're building a
privilege-escalation attack into the system. But if we do require
A-can-set-role-to-B, then things change as described above.
I suppose in theory we could check both A-can-set-role-to-B and
A-can-modify-this-table-as-A, but that feels pretty unprincipled. I
can't particularly see why A should need permission to perform an
action that is actually going to be performed as B; what makes sense
is to check that A can become B, and that B has permission to perform
the action, which is the state that this patch would create. I guess
there are other things we could do, too, like add replication
restriction or filtering capabilities specifically to address this
problem, or devise some other kind of new kind of permission system
around this, but I don't have a specific idea what that would look
like.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: running logical replication as the subscription owner
2023-03-03 16:02 running logical replication as the subscription owner Robert Haas <[email protected]>
2023-03-24 07:59 ` Re: running logical replication as the subscription owner Jeff Davis <[email protected]>
2023-03-24 14:00 ` Re: running logical replication as the subscription owner Robert Haas <[email protected]>
2023-03-24 16:58 ` Re: running logical replication as the subscription owner Mark Dilger <[email protected]>
2023-03-24 18:35 ` Re: running logical replication as the subscription owner Robert Haas <[email protected]>
@ 2023-03-25 09:24 ` Jelte Fennema <[email protected]>
0 siblings, 0 replies; 52+ messages in thread
From: Jelte Fennema @ 2023-03-25 09:24 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; pgsql-hackers; Andres Freund <[email protected]>; Noah Misch <[email protected]>
> As things stand today, if you think somebody's going to send you
> changes to tables other than the ones you intend to replicate, you
> could handle that by making sure that the user that owns the
> subscription only has permission to write to the tables that are
> expected to receive replicated data. It's a bit awkward to set up
> because you have to initially make the subscription owner a superuser
> and then later remove the superuser bit, so I think this is another
> argument for the pg_create_subscription patch posted elsewhere, but if
> you're a superuser yourself, you can do it. However, with this patch,
> that wouldn't work any more, because the permissions checks don't
> happen until after we've switched to the target role.
For my purposes I always trust the publisher, what I don't trust is
the table owners. But I can indeed imagine scenarios where that's the
other way around, and indeed you can protect against that currently,
but not with your new patch. That seems fairly easily solvable though.
+ if (!member_can_set_role(context->save_userid, userid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("role \"%s\" cannot SET ROLE to \"%s\"",
+ GetUserNameFromId(context->save_userid, false),
+ GetUserNameFromId(userid, false))));
If we don't throw an error here, but instead simply return, then the
current behaviour is preserved and people can manually configure
permissions to protect against an untrusted publisher. This would
still mean that the table owners can escalate privileges to the
subscription owner, but if that subscription owner actually has fewer
privileges than the table owner then you don't have that issue.
^ permalink raw reply [nested|flat] 52+ messages in thread
end of thread, other threads:[~2023-03-25 09:24 UTC | newest]
Thread overview: 52+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v9 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v3 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v8 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v6 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v8 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v5 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2020-07-14 00:15 [PATCH v1 1/2] Let ALTER TABLE exec routines deal with the relation Alvaro Herrera <[email protected]>
2023-03-03 16:02 running logical replication as the subscription owner Robert Haas <[email protected]>
2023-03-24 07:59 ` Re: running logical replication as the subscription owner Jeff Davis <[email protected]>
2023-03-24 14:00 ` Re: running logical replication as the subscription owner Robert Haas <[email protected]>
2023-03-24 16:58 ` Re: running logical replication as the subscription owner Mark Dilger <[email protected]>
2023-03-24 18:35 ` Re: running logical replication as the subscription owner Robert Haas <[email protected]>
2023-03-25 09:24 ` Re: running logical replication as the subscription owner Jelte Fennema <[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