public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
22+ 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; 22+ 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] 22+ messages in thread

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

* Re: Supply restore_command to pg_rewind via CLI argument
@ 2022-03-22 00:32 Andres Freund <[email protected]>
  2022-03-22 09:23 ` Re: Supply restore_command to pg_rewind via CLI argument Alexey Kondratov <[email protected]>
  0 siblings, 1 reply; 22+ messages in thread

From: Andres Freund @ 2022-03-22 00:32 UTC (permalink / raw)
  To: Andrey Borodin <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Michael Paquier <[email protected]>; Alexey Kondratov <[email protected]>; pgsql-hackers

Hi,

On 2021-11-05 15:10:29 +0500, Andrey Borodin wrote:
> > 4 нояб. 2021 г., в 17:55, Daniel Gustafsson <[email protected]> написал(а):
> > 
> > The patch no longer applies, can you submit a rebased version please?
> 
> Thanks, Daniel! PFA rebase.

Doesn't apply once more: http://cfbot.cputube.org/patch_37_3213.log

Greetings,

Andres Freund






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

* Re: Supply restore_command to pg_rewind via CLI argument
  2022-03-22 00:32 Re: Supply restore_command to pg_rewind via CLI argument Andres Freund <[email protected]>
@ 2022-03-22 09:23 ` Alexey Kondratov <[email protected]>
  0 siblings, 0 replies; 22+ messages in thread

From: Alexey Kondratov @ 2022-03-22 09:23 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Andrey Borodin <[email protected]>; Daniel Gustafsson <[email protected]>; Michael Paquier <[email protected]>; pgsql-hackers

Hi,

On Tue, Mar 22, 2022 at 3:32 AM Andres Freund <[email protected]> wrote:
>
> Doesn't apply once more: http://cfbot.cputube.org/patch_37_3213.log
>

Thanks for the reminder, a rebased version is attached.


Regards
-- 
Alexey Kondratov


Attachments:

  [text/x-patch] v4-0001-Allow-providing-restore_command-as-a-command-line.patch (11.4K, ../../CADfU8WyLNtBjwT8gFYuLj1M91vH8MgHknmaEAvUP=uER6sHE_g@mail.gmail.com/2-v4-0001-Allow-providing-restore_command-as-a-command-line.patch)
  download | inline diff:
From df56b5c7b882e781fdc0b92e7a83331f0baab094 Mon Sep 17 00:00:00 2001
From: Alexey Kondratov <[email protected]>
Date: Tue, 29 Jun 2021 17:17:47 +0300
Subject: [PATCH v4] Allow providing restore_command as a command line option
 to pg_rewind

This could be useful when postgres is usually run with
-c config_file=..., so the actual configuration and restore_command
is not inside $PGDATA/postgresql.conf.
---
 doc/src/sgml/ref/pg_rewind.sgml          | 19 +++++
 src/bin/pg_rewind/pg_rewind.c            | 45 ++++++-----
 src/bin/pg_rewind/t/001_basic.pl         |  1 +
 src/bin/pg_rewind/t/RewindTest.pm        | 95 ++++++++++++++----------
 src/test/perl/PostgreSQL/Test/Cluster.pm |  5 +-
 5 files changed, 106 insertions(+), 59 deletions(-)

diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml
index 33e6bb64ad..af75f35867 100644
--- a/doc/src/sgml/ref/pg_rewind.sgml
+++ b/doc/src/sgml/ref/pg_rewind.sgml
@@ -241,6 +241,25 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>-C <replaceable class="parameter">restore_command</replaceable></option></term>
+      <term><option>--target-restore-command=<replaceable class="parameter">restore_command</replaceable></option></term>
+      <listitem>
+       <para>
+        Specifies the <varname>restore_command</varname> to use for retrieving
+        WAL files from the WAL archive if these files are no longer available
+        in the <filename>pg_wal</filename> directory of the target cluster.
+       </para>
+       <para>
+        If <varname>restore_command</varname> is already set in
+        <filename>postgresql.conf</filename>, you can provide the
+        <option>--restore-target-wal</option> option instead. If both options
+        are provided, then <option>--target-restore-command</option>
+        will be used.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--debug</option></term>
       <listitem>
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index b39b5c1aac..9aca041425 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -85,21 +85,22 @@ usage(const char *progname)
 	printf(_("%s resynchronizes a PostgreSQL cluster with another copy of the cluster.\n\n"), progname);
 	printf(_("Usage:\n  %s [OPTION]...\n\n"), progname);
 	printf(_("Options:\n"));
-	printf(_("  -c, --restore-target-wal       use restore_command in target configuration to\n"
-			 "                                 retrieve WAL files from archives\n"));
-	printf(_("  -D, --target-pgdata=DIRECTORY  existing data directory to modify\n"));
-	printf(_("      --source-pgdata=DIRECTORY  source data directory to synchronize with\n"));
-	printf(_("      --source-server=CONNSTR    source server to synchronize with\n"));
-	printf(_("  -n, --dry-run                  stop before modifying anything\n"));
-	printf(_("  -N, --no-sync                  do not wait for changes to be written\n"
-			 "                                 safely to disk\n"));
-	printf(_("  -P, --progress                 write progress messages\n"));
-	printf(_("  -R, --write-recovery-conf      write configuration for replication\n"
-			 "                                 (requires --source-server)\n"));
-	printf(_("      --debug                    write a lot of debug messages\n"));
-	printf(_("      --no-ensure-shutdown       do not automatically fix unclean shutdown\n"));
-	printf(_("  -V, --version                  output version information, then exit\n"));
-	printf(_("  -?, --help                     show this help, then exit\n"));
+	printf(_("  -c, --restore-target-wal              use restore_command in target configuration to\n"
+			 "                                        retrieve WAL files from archives\n"));
+	printf(_("  -C, --target-restore-command=COMMAND  target WAL restore_command\n"));
+	printf(_("  -D, --target-pgdata=DIRECTORY         existing data directory to modify\n"));
+	printf(_("      --source-pgdata=DIRECTORY         source data directory to synchronize with\n"));
+	printf(_("      --source-server=CONNSTR           source server to synchronize with\n"));
+	printf(_("  -n, --dry-run                         stop before modifying anything\n"));
+	printf(_("  -N, --no-sync                         do not wait for changes to be written\n"
+			 "                                        safely to disk\n"));
+	printf(_("  -P, --progress                        write progress messages\n"));
+	printf(_("  -R, --write-recovery-conf             write configuration for replication\n"
+			 "                                        (requires --source-server)\n"));
+	printf(_("      --debug                           write a lot of debug messages\n"));
+	printf(_("      --no-ensure-shutdown              do not automatically fix unclean shutdown\n"));
+	printf(_("  -V, --version                         output version information, then exit\n"));
+	printf(_("  -?, --help                            show this help, then exit\n"));
 	printf(_("\nReport bugs to <%s>.\n"), PACKAGE_BUGREPORT);
 	printf(_("%s home page: <%s>\n"), PACKAGE_NAME, PACKAGE_URL);
 }
@@ -117,6 +118,7 @@ main(int argc, char **argv)
 		{"no-ensure-shutdown", no_argument, NULL, 4},
 		{"version", no_argument, NULL, 'V'},
 		{"restore-target-wal", no_argument, NULL, 'c'},
+		{"target-restore-command", required_argument, NULL, 'C'},
 		{"dry-run", no_argument, NULL, 'n'},
 		{"no-sync", no_argument, NULL, 'N'},
 		{"progress", no_argument, NULL, 'P'},
@@ -157,7 +159,7 @@ main(int argc, char **argv)
 		}
 	}
 
-	while ((c = getopt_long(argc, argv, "cD:nNPR", long_options, &option_index)) != -1)
+	while ((c = getopt_long(argc, argv, "cC:D:nNPR", long_options, &option_index)) != -1)
 	{
 		switch (c)
 		{
@@ -169,6 +171,11 @@ main(int argc, char **argv)
 				restore_wal = true;
 				break;
 
+			case 'C':
+				restore_wal = true;
+				restore_command = pg_strdup(optarg);
+				break;
+
 			case 'P':
 				showprogress = true;
 				break;
@@ -1020,7 +1027,11 @@ getRestoreCommand(const char *argv0)
 				cmd_output[MAXPGPATH];
 	PQExpBuffer postgres_cmd;
 
-	if (!restore_wal)
+	/*
+	 * Take restore_command from the postgresql.conf only if it is not already
+	 * provided as a command line option.
+	 */
+	if (!restore_wal || restore_command != NULL)
 		return;
 
 	/* find postgres executable */
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index db9201f38e..3c395ece12 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -190,5 +190,6 @@ in primary, before promotion
 run_test('local');
 run_test('remote');
 run_test('archive');
+run_test('archive_cli');
 
 done_testing();
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
index 5651602858..335f1a72ad 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -208,6 +208,58 @@ sub promote_standby
 	return;
 }
 
+sub run_pg_rewind_archive
+{
+	my $test_mode = shift;
+	my $restore_command;
+	my $cli_option = '--restore-target-wal';
+
+	# Remove the existing archive directory and move all WAL
+	# segments from the old primary to the archives.  These
+	# will be used by pg_rewind.
+	rmtree($node_primary->archive_dir);
+	PostgreSQL::Test::RecursiveCopy::copypath($node_primary->data_dir . '/pg_wal',
+		$node_primary->archive_dir);
+
+	# Fast way to remove entire directory content
+	rmtree($node_primary->data_dir . '/pg_wal');
+	mkdir($node_primary->data_dir . '/pg_wal');
+
+	# Make sure that directories have the right umask as this is
+	# required by a follow-up check on permissions, and better
+	# safe than sorry.
+	chmod(0700, $node_primary->archive_dir);
+	chmod(0700, $node_primary->data_dir . '/pg_wal');
+
+	# Add appropriate restore_command to the target cluster
+	$restore_command = $node_primary->enable_restoring($node_primary, 0);
+
+	# In archive_cli mode pass restore_command as a command line option
+	# instead of taking it from postgresql.conf
+	if ($test_mode eq 'archive_cli') {
+		$cli_option = "--target-restore-command=$restore_command";
+	}
+
+	# Stop the new primary and be ready to perform the rewind.
+	$node_standby->stop;
+
+	# Note the use of --no-ensure-shutdown here.  WAL files are
+	# gone in this mode and the primary has been stopped
+	# gracefully already.
+	command_ok(
+		[
+			'pg_rewind',
+			'--debug',
+			'--source-pgdata=' . $node_standby->data_dir,
+			'--target-pgdata=' . $node_primary->data_dir,
+			'--no-sync',
+			'--no-ensure-shutdown',
+			$cli_option
+		],
+		"pg_rewind $test_mode");
+
+}
+
 sub run_pg_rewind
 {
 	my $test_mode       = shift;
@@ -219,7 +271,7 @@ sub run_pg_rewind
 	# Append the rewind-specific role to the connection string.
 	$standby_connstr = "$standby_connstr user=rewind_user";
 
-	if ($test_mode eq 'archive')
+	if ($test_mode eq 'archive' or $test_mode eq 'archive_cli')
 	{
 		# pg_rewind is tested with --restore-target-wal by moving all
 		# WAL files to a secondary location.  Note that this leads to
@@ -292,50 +344,13 @@ sub run_pg_rewind
 		$node_standby->safe_psql('postgres',
 			"ALTER ROLE rewind_user WITH REPLICATION;");
 	}
-	elsif ($test_mode eq "archive")
+	elsif ($test_mode eq "archive" or $test_mode eq "archive_cli")
 	{
-
 		# Do rewind using a local pgdata as source and specified
 		# directory with target WAL archive.  The old primary has
 		# to be stopped at this point.
 
-		# Remove the existing archive directory and move all WAL
-		# segments from the old primary to the archives.  These
-		# will be used by pg_rewind.
-		rmtree($node_primary->archive_dir);
-		PostgreSQL::Test::RecursiveCopy::copypath($node_primary->data_dir . "/pg_wal",
-			$node_primary->archive_dir);
-
-		# Fast way to remove entire directory content
-		rmtree($node_primary->data_dir . "/pg_wal");
-		mkdir($node_primary->data_dir . "/pg_wal");
-
-		# Make sure that directories have the right umask as this is
-		# required by a follow-up check on permissions, and better
-		# safe than sorry.
-		chmod(0700, $node_primary->archive_dir);
-		chmod(0700, $node_primary->data_dir . "/pg_wal");
-
-		# Add appropriate restore_command to the target cluster
-		$node_primary->enable_restoring($node_primary, 0);
-
-		# Stop the new primary and be ready to perform the rewind.
-		$node_standby->stop;
-
-		# Note the use of --no-ensure-shutdown here.  WAL files are
-		# gone in this mode and the primary has been stopped
-		# gracefully already.
-		command_ok(
-			[
-				'pg_rewind',
-				"--debug",
-				"--source-pgdata=$standby_pgdata",
-				"--target-pgdata=$primary_pgdata",
-				"--no-sync",
-				"--no-ensure-shutdown",
-				"--restore-target-wal"
-			],
-			'pg_rewind archive');
+		run_pg_rewind_archive($test_mode);
 	}
 	else
 	{
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index e7b9161137..51e289aaa0 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -1071,7 +1071,8 @@ primary_conninfo='$root_connstr'
 	return;
 }
 
-# Internal routine to enable archive recovery command on a standby node
+# Internal routine to enable archive recovery command on a standby node.
+# Returns generated restore_command.
 sub enable_restoring
 {
 	my ($self, $root_node, $standby) = @_;
@@ -1104,7 +1105,7 @@ restore_command = '$copy_command'
 	{
 		$self->set_recovery_mode();
 	}
-	return;
+	return $copy_command;
 }
 
 =pod

base-commit: f5576a21b0778f275d7418f6f7a44d9400ee90aa
-- 
2.32.0



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

end of thread, other threads:[~2022-03-22 09:23 UTC | newest]

Thread overview: 22+ 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-03-22 00:32 Re: Supply restore_command to pg_rewind via CLI argument Andres Freund <[email protected]>
2022-03-22 09:23 ` Re: Supply restore_command to pg_rewind via CLI argument Alexey Kondratov <[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