public inbox for [email protected]  

help / color / mirror / Atom feed

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

* Re: [PATCH] Add native windows on arm64 support
@ 2023-01-17 09:19  Niyas Sait <[email protected]>
  0 siblings, 0 replies; 21+ messages in thread

From: Niyas Sait @ 2023-01-17 09:19 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Andres Freund <[email protected]>; Ian Lawrence Barwick <[email protected]>; Tom Lane <[email protected]>; Thomas Munro <[email protected]>; Julien Rouhaud <[email protected]>; PostgreSQL Hackers <[email protected]>


On 16/12/2022 10:52, Niyas Sait wrote:
> On 12/12/2022 23:56, Michael Paquier wrote:
>> On Mon, Dec 12, 2022 at 01:38:37PM +0000, Niyas Sait wrote:
>>> On 05/12/2022 18:14, Andres Freund wrote:
>>> I think the old build system specific part is really minimal in the 
>>> patch. I
>>> can strip out those if that's preferred.
>> Removing all the changes from src/tools/msvc/ is an approach that
>> works for me.
>>
> 
> I've attached a new version (v7) that removes changes to support old 
> MSVC build system.


Hello,

Can I get review for the latest patch please ?

-- 
Niyas






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

end of thread, other threads:[~2023-01-17 09:19 UTC | newest]

Thread overview: 21+ 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]>
2023-01-17 09:19 Re: [PATCH] Add native windows on arm64 support Niyas Sait <[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