public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
24+ messages / 3 participants
[nested] [flat]

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* [PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
@ 2020-08-11 06:41 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Justin Pryzby @ 2020-08-11 06:41 UTC (permalink / raw)

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized option.
---
 src/backend/commands/indexcmds.c | 20 +++++++++++---------
 src/backend/nodes/copyfuncs.c    |  1 -
 src/backend/nodes/equalfuncs.c   |  1 -
 src/backend/parser/gram.y        | 16 ++++++++++++----
 src/backend/tcop/utility.c       |  6 +++---
 src/include/nodes/parsenodes.h   |  2 +-
 6 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a22b32fc74..097d8720a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
  */
 struct ReindexIndexCallbackState
 {
-	bool		concurrent;		/* flag from statement */
+	bool		concurrent;		/* flag from statement XXX */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, stmt->options);
 	else
 		reindex_index(indOid, false, persistence,
@@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt)
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, stmt->options);
 
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (stmt->options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		/* functions in indexes may want a snapshot set */
 		PushActiveSnapshot(GetTransactionSnapshot());
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid, stmt->options);
 			/* ReindexRelationConcurrently() does the verbose output */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..a67e637c37 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_STRING_FIELD(name);
 	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..efcb23c12e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_STRING_FIELD(name);
 	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b909b161a6..ac489c03c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c48983a5de..639b6fce74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 				stmt->options &= ~REINDEXOPT_VERBOSE;
 		}
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+			stmt->options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt);
+				if (stmt->options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 295c2802a4..ba685139ef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt
 /* Reindex options */
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
+#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
 	const char *name;			/* name of database to reindex */
 	int			options;		/* Reindex options flags */
 	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.17.0


--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v22-0003-Allow-REINDEX-to-change-tablespace.patch"



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

* Progress report removal of temp files and temp relation files using ereport_startup_progress
@ 2022-04-30 05:37 Bharath Rupireddy <[email protected]>
  2022-05-02 12:56 ` Re: Progress report removal of temp files and temp relation files using ereport_startup_progress Ashutosh Bapat <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Bharath Rupireddy @ 2022-04-30 05:37 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>

Hi,

At times, there can be many temp files (under pgsql_tmp) and temp
relation files (under removal which after crash may take longer during
which users have no clue about what's going on in the server before it
comes up online.

Here's a proposal to use ereport_startup_progress to report the
progress of the file removal.

Thoughts?

Regards,
Bharath Rupireddy.


Attachments:

  [application/octet-stream] v1-0001-Progress-report-removal-of-temp-files-and-temp-re.patch (1.9K, ../../CALj2ACWh+hgE4U2W_zKUVLjLBaonyMCOKDEB6oFLP91y3AccWw@mail.gmail.com/2-v1-0001-Progress-report-removal-of-temp-files-and-temp-re.patch)
  download | inline diff:
From 9f9da2ad7d1bc433bbef0ed44b2f37454d968ec2 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Sat, 30 Apr 2022 05:35:22 +0000
Subject: [PATCH v1] Progress report removal of temp files and temp relation
 files

At times, there can be many temp files (under pgsql_tmp) and temp
relation files (under  removal which after crash may take longer
during which users have no clue about what's going on in the
server before it comes up online. This patch uses
ereport_startup_progress to report the progress of the file
removal.
---
 src/backend/storage/file/fd.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 24704b6a02..08c333e514 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -3156,6 +3156,12 @@ RemovePgTempFiles(void)
 	DIR		   *spc_dir;
 	struct dirent *spc_de;
 
+	/*
+	 * Prepare to report progress of the temporary and temporary relation files
+	 * removal phase.
+	 */
+	begin_startup_progress_phase();
+
 	/*
 	 * First process temp files in pg_default ($PGDATA/base)
 	 */
@@ -3229,6 +3235,9 @@ RemovePgTempFilesInDir(const char *tmpdirname, bool missing_ok, bool unlink_all)
 		snprintf(rm_path, sizeof(rm_path), "%s/%s",
 				 tmpdirname, temp_de->d_name);
 
+		ereport_startup_progress("removing temporary files under pgsql_tmp directory, elapsed time: %ld.%02d s, current file: %s",
+								 rm_path);
+
 		if (unlink_all ||
 			strncmp(temp_de->d_name,
 					PG_TEMP_FILE_PREFIX,
@@ -3319,6 +3328,9 @@ RemovePgTempRelationFilesInDbspace(const char *dbspacedirname)
 		snprintf(rm_path, sizeof(rm_path), "%s/%s",
 				 dbspacedirname, de->d_name);
 
+		ereport_startup_progress("removing temporary relation files under pg_tblspc directory, elapsed time: %ld.%02d s, current file: %s",
+								 rm_path);
+
 		if (unlink(rm_path) < 0)
 			ereport(LOG,
 					(errcode_for_file_access(),
-- 
2.25.1



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

* Re: Progress report removal of temp files and temp relation files using ereport_startup_progress
  2022-04-30 05:37 Progress report removal of temp files and temp relation files using ereport_startup_progress Bharath Rupireddy <[email protected]>
@ 2022-05-02 12:56 ` Ashutosh Bapat <[email protected]>
  2022-05-05 06:41   ` Re: Progress report removal of temp files and temp relation files using ereport_startup_progress Bharath Rupireddy <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Ashutosh Bapat @ 2022-05-02 12:56 UTC (permalink / raw)
  To: Bharath Rupireddy <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

Hi Bharath,


On Sat, Apr 30, 2022 at 11:08 AM Bharath Rupireddy
<[email protected]> wrote:
>
> Hi,
>
> At times, there can be many temp files (under pgsql_tmp) and temp
> relation files (under removal which after crash may take longer during
> which users have no clue about what's going on in the server before it
> comes up online.
>
> Here's a proposal to use ereport_startup_progress to report the
> progress of the file removal.
>
> Thoughts?

The patch looks good to me.

With this patch, the user would at least know which directory is being
scanned and how much time has elapsed. It would be better to know how
much work is remaining. I could not find a way to estimate the number
of files in the directory so that we can extrapolate elapsed time and
estimate the remaining time. Well, we could loop the output of
opendir() twice, first to estimate and then for the actual work. This
might actually work, if the time to delete all the files is very high
compared to the time it takes to scan all the files/directories.

Another possibility is to scan the sorted output of opendir() thus
using the current file name to estimate remaining files in a very
crude and inaccurate way. That doesn't look attractive either. I can't
think of any better way to estimate the remaining time.

But at least with this patch, a user knows which files have been
deleted, guessing how far, in the directory structure, the process has
reached. S/he can then take a look at the remaining contents of the
directory to estimate how much it should wait.

-- 
Best Wishes,
Ashutosh Bapat





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

* Re: Progress report removal of temp files and temp relation files using ereport_startup_progress
  2022-04-30 05:37 Progress report removal of temp files and temp relation files using ereport_startup_progress Bharath Rupireddy <[email protected]>
  2022-05-02 12:56 ` Re: Progress report removal of temp files and temp relation files using ereport_startup_progress Ashutosh Bapat <[email protected]>
@ 2022-05-05 06:41   ` Bharath Rupireddy <[email protected]>
  2022-08-02 06:22     ` Re: Progress report removal of temp files and temp relation files using ereport_startup_progress Bharath Rupireddy <[email protected]>
  0 siblings, 1 reply; 24+ messages in thread

From: Bharath Rupireddy @ 2022-05-05 06:41 UTC (permalink / raw)
  To: Ashutosh Bapat <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Mon, May 2, 2022 at 6:26 PM Ashutosh Bapat
<[email protected]> wrote:
>
> Hi Bharath,
>
>
> On Sat, Apr 30, 2022 at 11:08 AM Bharath Rupireddy
> <[email protected]> wrote:
> >
> > Hi,
> >
> > At times, there can be many temp files (under pgsql_tmp) and temp
> > relation files (under removal which after crash may take longer during
> > which users have no clue about what's going on in the server before it
> > comes up online.
> >
> > Here's a proposal to use ereport_startup_progress to report the
> > progress of the file removal.
> >
> > Thoughts?
>
> The patch looks good to me.
>
> With this patch, the user would at least know which directory is being
> scanned and how much time has elapsed.

There's a problem with the patch, the timeout mechanism isn't being
used by the postmaster process. Postmaster doesn't
InitializeTimeouts() and doesn't register STARTUP_PROGRESS_TIMEOUT, I
tried to make postmaster do that (attached a v2 patch) but make check
fails.

Now, I'm thinking if it's a good idea to let postmaster use timeouts at all?

> It would be better to know how
> much work is remaining. I could not find a way to estimate the number
> of files in the directory so that we can extrapolate elapsed time and
> estimate the remaining time. Well, we could loop the output of
> opendir() twice, first to estimate and then for the actual work. This
> might actually work, if the time to delete all the files is very high
> compared to the time it takes to scan all the files/directories.
>
> Another possibility is to scan the sorted output of opendir() thus
> using the current file name to estimate remaining files in a very
> crude and inaccurate way. That doesn't look attractive either. I can't
> think of any better way to estimate the remaining time.

I think 'how much work/how many files remaining to process' is a
generic problem, for instance, snapshot, mapping files, old WAL file
processing and so on. I don't think we can do much about it.

> But at least with this patch, a user knows which files have been
> deleted, guessing how far, in the directory structure, the process has
> reached. S/he can then take a look at the remaining contents of the
> directory to estimate how much it should wait.

Not sure we will be able to use the timeout mechanism within
postmaster. Another idea is to have a generic GUC something like
log_file_processing_traffic = {none, medium, high} (similar idea is
proposed for WAL files processing while replaying/recovering at [1]),
default being none, when set to medium a log message gets emitted for
every say 128 or 256 (just a random number) files processed. when set
to high, log messages get emitted for every file processed (too
verbose). I think this generic GUC log_file_processing_traffic can be
used in many other file processing areas.

Thoughts?

[1] https://www.postgresql.org/message-id/CALj2ACVnhbx4pLZepvdqOfeOekvZXJ2F%3DwJeConGzok%2B6kgCVA%40mail...

Regards,
Bharath Rupireddy.


Attachments:

  [application/octet-stream] v2-0001-pgsql_tmp-ereport_startup_progress.patch (3.6K, ../../CALj2ACW-ELOF5JT2zPavs95wbZ0BrLPrqvSZ7Ac+pjxCkmXtEQ@mail.gmail.com/2-v2-0001-pgsql_tmp-ereport_startup_progress.patch)
  download | inline diff:
From 059099c469c40ee9d91a3b37b8960b36557e6030 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Wed, 4 May 2022 14:17:24 +0000
Subject: [PATCH v2] pgsql_tmp ereport_startup_progress

---
 src/backend/postmaster/postmaster.c | 11 +++++++++++
 src/backend/storage/file/fd.c       | 15 +++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 964a56dec4..261a5a5911 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -116,6 +116,7 @@
 #include "postmaster/interrupt.h"
 #include "postmaster/pgarch.h"
 #include "postmaster/postmaster.h"
+#include "postmaster/startup.h"
 #include "postmaster/syslogger.h"
 #include "replication/logicallauncher.h"
 #include "replication/walsender.h"
@@ -689,6 +690,11 @@ PostmasterMain(int argc, char *argv[])
 	pqsignal_pm(SIGXFSZ, SIG_IGN);	/* ignored */
 #endif
 
+	InitializeTimeouts();		/* establishes SIGALRM handler */
+
+	RegisterTimeout(STARTUP_PROGRESS_TIMEOUT,
+					startup_progress_timeout_handler);
+
 	/*
 	 * Options setup
 	 */
@@ -1104,6 +1110,9 @@ PostmasterMain(int argc, char *argv[])
 	/* Write out nondefault GUC settings for child processes to use */
 	write_nondefault_variables(PGC_POSTMASTER);
 
+	/* Prepare to report progress of the temporary files removal phase */
+	begin_startup_progress_phase();
+
 	/*
 	 * Clean out the temp directory used to transmit parameters to child
 	 * processes (see internal_forkexec, below).  We must do this before
@@ -1113,6 +1122,8 @@ PostmasterMain(int argc, char *argv[])
 	 * conflicting Postgres processes in this data directory.
 	 */
 	RemovePgTempFilesInDir(PG_TEMP_FILES_DIR, true, false);
+
+	disable_timeout(STARTUP_PROGRESS_TIMEOUT, false);
 #endif
 
 	/*
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 24704b6a02..c793303bd8 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -102,6 +102,7 @@
 #include "storage/ipc.h"
 #include "utils/guc.h"
 #include "utils/resowner_private.h"
+#include "utils/timeout.h"
 
 /* Define PG_FLUSH_DATA_WORKS if we have an implementation for pg_flush_data */
 #if defined(HAVE_SYNC_FILE_RANGE)
@@ -3156,6 +3157,12 @@ RemovePgTempFiles(void)
 	DIR		   *spc_dir;
 	struct dirent *spc_de;
 
+	/*
+	 * Prepare to report progress of the temporary and temporary relation files
+	 * removal phase.
+	 */
+	begin_startup_progress_phase();
+
 	/*
 	 * First process temp files in pg_default ($PGDATA/base)
 	 */
@@ -3185,6 +3192,8 @@ RemovePgTempFiles(void)
 
 	FreeDir(spc_dir);
 
+	disable_timeout(STARTUP_PROGRESS_TIMEOUT, false);
+
 	/*
 	 * In EXEC_BACKEND case there is a pgsql_tmp directory at the top level of
 	 * DataDir as well.  However, that is *not* cleaned here because doing so
@@ -3229,6 +3238,9 @@ RemovePgTempFilesInDir(const char *tmpdirname, bool missing_ok, bool unlink_all)
 		snprintf(rm_path, sizeof(rm_path), "%s/%s",
 				 tmpdirname, temp_de->d_name);
 
+		ereport_startup_progress("removing temporary files under pgsql_tmp directory, elapsed time: %ld.%02d s, current file: %s",
+								 rm_path);
+
 		if (unlink_all ||
 			strncmp(temp_de->d_name,
 					PG_TEMP_FILE_PREFIX,
@@ -3319,6 +3331,9 @@ RemovePgTempRelationFilesInDbspace(const char *dbspacedirname)
 		snprintf(rm_path, sizeof(rm_path), "%s/%s",
 				 dbspacedirname, de->d_name);
 
+		ereport_startup_progress("removing temporary relation files under pg_tblspc directory, elapsed time: %ld.%02d s, current file: %s",
+								 rm_path);
+
 		if (unlink(rm_path) < 0)
 			ereport(LOG,
 					(errcode_for_file_access(),
-- 
2.25.1



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

* Re: Progress report removal of temp files and temp relation files using ereport_startup_progress
  2022-04-30 05:37 Progress report removal of temp files and temp relation files using ereport_startup_progress Bharath Rupireddy <[email protected]>
  2022-05-02 12:56 ` Re: Progress report removal of temp files and temp relation files using ereport_startup_progress Ashutosh Bapat <[email protected]>
  2022-05-05 06:41   ` Re: Progress report removal of temp files and temp relation files using ereport_startup_progress Bharath Rupireddy <[email protected]>
@ 2022-08-02 06:22     ` Bharath Rupireddy <[email protected]>
  0 siblings, 0 replies; 24+ messages in thread

From: Bharath Rupireddy @ 2022-08-02 06:22 UTC (permalink / raw)
  To: Ashutosh Bapat <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Thu, May 5, 2022 at 12:11 PM Bharath Rupireddy
<[email protected]> wrote:
>
> On Mon, May 2, 2022 at 6:26 PM Ashutosh Bapat
> <[email protected]> wrote:
> >
> > Hi Bharath,
> >
> >
> > On Sat, Apr 30, 2022 at 11:08 AM Bharath Rupireddy
> > <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > At times, there can be many temp files (under pgsql_tmp) and temp
> > > relation files (under removal which after crash may take longer during
> > > which users have no clue about what's going on in the server before it
> > > comes up online.
> > >
> > > Here's a proposal to use ereport_startup_progress to report the
> > > progress of the file removal.
> > >
> > > Thoughts?
> >
> > The patch looks good to me.
> >
> > With this patch, the user would at least know which directory is being
> > scanned and how much time has elapsed.
>
> There's a problem with the patch, the timeout mechanism isn't being
> used by the postmaster process. Postmaster doesn't
> InitializeTimeouts() and doesn't register STARTUP_PROGRESS_TIMEOUT, I
> tried to make postmaster do that (attached a v2 patch) but make check
> fails.
>
> Now, I'm thinking if it's a good idea to let postmaster use timeouts at all?

Here's the v3 patch, which adds progress reports for temp file removal
under the pgsql_tmp directory and temporary relation files under the
pg_tblspc directory, regression tests pass with it.

Regards,
Bharath Rupireddy.


Attachments:

  [application/x-patch] v3-0001-Progress-report-removal-of-temp-files-and-temp-re.patch (4.9K, ../../CALj2ACWeUFhhnDJKm6R5YxCsF4K7aB2pmRMvqP0BVTxdyce3EA@mail.gmail.com/2-v3-0001-Progress-report-removal-of-temp-files-and-temp-re.patch)
  download | inline diff:
From c31492191ffcd6222d7e42917cbd928351623780 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Tue, 2 Aug 2022 05:10:06 +0000
Subject: [PATCH v3] Progress report removal of temp files and temp relation
 files

At times, there can be many temp files (under pgsql_tmp) and temp
relation files (under  removal which after crash may take longer
during which users have no clue about what's going on in the
server before it comes up online. This patch uses
ereport_startup_progress to report the progress of the file
removal.
---
 src/backend/postmaster/postmaster.c | 10 +++++++++-
 src/backend/storage/file/fd.c       | 13 +++++++++++++
 src/backend/utils/misc/timeout.c    | 11 +++++++++--
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index e541b16bdb..40b53276be 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -116,6 +116,7 @@
 #include "postmaster/interrupt.h"
 #include "postmaster/pgarch.h"
 #include "postmaster/postmaster.h"
+#include "postmaster/startup.h"
 #include "postmaster/syslogger.h"
 #include "replication/logicallauncher.h"
 #include "replication/walsender.h"
@@ -653,7 +654,6 @@ PostmasterMain(int argc, char *argv[])
 	pqsignal_pm(SIGINT, pmdie); /* send SIGTERM and shut down */
 	pqsignal_pm(SIGQUIT, pmdie);	/* send SIGQUIT and die */
 	pqsignal_pm(SIGTERM, pmdie);	/* wait for children and shut down */
-	pqsignal_pm(SIGALRM, SIG_IGN);	/* ignored */
 	pqsignal_pm(SIGPIPE, SIG_IGN);	/* ignored */
 	pqsignal_pm(SIGUSR1, sigusr1_handler);	/* message from child process */
 	pqsignal_pm(SIGUSR2, dummy_handler);	/* unused, reserve for children */
@@ -688,6 +688,11 @@ PostmasterMain(int argc, char *argv[])
 	pqsignal_pm(SIGXFSZ, SIG_IGN);	/* ignored */
 #endif
 
+	InitializeTimeouts();	/* establishes SIGALRM handler */
+
+	RegisterTimeout(STARTUP_PROGRESS_TIMEOUT,
+					startup_progress_timeout_handler);
+
 	/*
 	 * Options setup
 	 */
@@ -1119,6 +1124,9 @@ PostmasterMain(int argc, char *argv[])
 	/* Write out nondefault GUC settings for child processes to use */
 	write_nondefault_variables(PGC_POSTMASTER);
 
+	/* Prepare to report progress of the temporary files removal phase */
+	begin_startup_progress_phase();
+
 	/*
 	 * Clean out the temp directory used to transmit parameters to child
 	 * processes (see internal_forkexec, below).  We must do this before
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index f904f60c08..37c13a60ba 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -102,6 +102,7 @@
 #include "storage/ipc.h"
 #include "utils/guc.h"
 #include "utils/resowner_private.h"
+#include "utils/timeout.h"
 
 /* Define PG_FLUSH_DATA_WORKS if we have an implementation for pg_flush_data */
 #if defined(HAVE_SYNC_FILE_RANGE)
@@ -3093,6 +3094,12 @@ RemovePgTempFiles(void)
 	DIR		   *spc_dir;
 	struct dirent *spc_de;
 
+	/*
+	 * Prepare to report progress of the temporary and temporary relation files
+	 * removal phase.
+	 */
+	begin_startup_progress_phase();
+
 	/*
 	 * First process temp files in pg_default ($PGDATA/base)
 	 */
@@ -3166,6 +3173,9 @@ RemovePgTempFilesInDir(const char *tmpdirname, bool missing_ok, bool unlink_all)
 		snprintf(rm_path, sizeof(rm_path), "%s/%s",
 				 tmpdirname, temp_de->d_name);
 
+		ereport_startup_progress("removing temporary files under pgsql_tmp directory, elapsed time: %ld.%02d s, current file: %s",
+								 rm_path);
+
 		if (unlink_all ||
 			strncmp(temp_de->d_name,
 					PG_TEMP_FILE_PREFIX,
@@ -3256,6 +3266,9 @@ RemovePgTempRelationFilesInDbspace(const char *dbspacedirname)
 		snprintf(rm_path, sizeof(rm_path), "%s/%s",
 				 dbspacedirname, de->d_name);
 
+		ereport_startup_progress("removing temporary relation files under pg_tblspc directory, elapsed time: %ld.%02d s, current file: %s",
+								 rm_path);
+
 		if (unlink(rm_path) < 0)
 			ereport(LOG,
 					(errcode_for_file_access(),
diff --git a/src/backend/utils/misc/timeout.c b/src/backend/utils/misc/timeout.c
index 6f5e08bc30..cc4b9d6ad7 100644
--- a/src/backend/utils/misc/timeout.c
+++ b/src/backend/utils/misc/timeout.c
@@ -16,6 +16,7 @@
 
 #include <sys/time.h>
 
+#include "libpq/pqsignal.h"
 #include "miscadmin.h"
 #include "storage/proc.h"
 #include "utils/timeout.h"
@@ -375,8 +376,11 @@ handle_sig_alarm(SIGNAL_ARGS)
 	/*
 	 * SIGALRM is always cause for waking anything waiting on the process
 	 * latch.
+	 *
+	 * Postmaster has no latch associated with it.
 	 */
-	SetLatch(MyLatch);
+	if (MyLatch)
+		SetLatch(MyLatch);
 
 	/*
 	 * Always reset signal_pending, even if !alarm_enabled, since indeed no
@@ -494,7 +498,10 @@ InitializeTimeouts(void)
 	all_timeouts_initialized = true;
 
 	/* Now establish the signal handler */
-	pqsignal(SIGALRM, handle_sig_alarm);
+	if (IsPostmasterEnvironment)
+		pqsignal_pm(SIGALRM, handle_sig_alarm);
+	else
+		pqsignal(SIGALRM, handle_sig_alarm);
 }
 
 /*
-- 
2.34.1



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

end of thread, other threads:[~2022-08-02 06:22 UTC | newest]

Thread overview: 24+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2020-08-11 06:41 [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. Justin Pryzby <[email protected]>
2022-04-30 05:37 Progress report removal of temp files and temp relation files using ereport_startup_progress Bharath Rupireddy <[email protected]>
2022-05-02 12:56 ` Re: Progress report removal of temp files and temp relation files using ereport_startup_progress Ashutosh Bapat <[email protected]>
2022-05-05 06:41   ` Re: Progress report removal of temp files and temp relation files using ereport_startup_progress Bharath Rupireddy <[email protected]>
2022-08-02 06:22     ` Re: Progress report removal of temp files and temp relation files using ereport_startup_progress Bharath Rupireddy <[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