public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v22 2/5] Deprecate ReindexStmt->concurrent..
2+ 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; 2+ 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] 2+ messages in thread
* Re: Remove MSVC scripts from the tree
@ 2023-11-08 14:47 Andrew Dunstan <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Dunstan @ 2023-11-08 14:47 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; Michael Paquier <[email protected]>; +Cc: Postgres hackers <[email protected]>
On 2023-11-08 We 03:41, Peter Eisentraut wrote:
> On 12.10.23 07:23, Michael Paquier wrote:
>>> - Is src/backend/utils/README.Gen_dummy_probes still correct after
>>> this?
>>> AFAICT, the Perl-based MSVC build system uses Gen_dummy_probes.pl,
>>> but the
>>> meson build uses Gen_dummy_probes.sed even on Windows. Is that
>>> correct,
>>> intended?
>> Interesting point. This may depend on the environment at the end? As
>> far as I can see, sed is currently a hard requirement in the meson
>> build and we'd fail if the command cannot be used. The buildfarm
>> machines that test meson are able to find sed, making
>> Gen_dummy_probes.pl not necessary:
>> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=drongo&dt=2023-10-11%2020%3A21%3A1...
>>
>>
>> So the $1000 question is: could there be a point in keeping the perl
>> script around if sed cannot be found? The buildfarm coverage is
>> currently saying no thanks to chocolatey, at least. The VM images
>> compiled by Andres for the CI seem to have the same opinion.
>
> I don't think we should rely on sed being there on Windows. Maybe
> it's true now on the handful of buildfarm/CI machines and early
> adopters, but do we have any indication that that is systematic or
> just an accident?
>
> Since we definitely require Perl now, we could just as well use the
> Perl script and avoid this issue.
>
> Attached is a Perl version of the sed script, converted by hand (so
> not the super-verbose s2p thing). It's basically just the sed script
> with semicolons added and the backslashes in the regular expressions
> moved around. I think we could use something like that for all
> platforms now.
I think it's alright, but please don't use literal tabs, use \t, even in
a character class.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2023-11-08 14:47 UTC | newest]
Thread overview: 2+ 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]>
2023-11-08 14:47 Re: Remove MSVC scripts from the tree Andrew Dunstan <[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