Re: Confusing error message for REINDEX TABLE CONCURRENTLY
On Wed, 5 Jun 2019 at 18:11, Michael Paquier wrote: > > On Tue, Jun 04, 2019 at 11:26:44AM -0700, Ashwin Agrawal wrote: > > The variable is used in else scope hence I moved it there. But yes its > > removed completely for this scope. > > Thanks for updating the patch. It does its job by having one separate > message for the concurrent and the non-concurrent cases as discussed. > David, what do you think? Perhaps you would like to commit it > yourself? Thanks. I've just pushed this with some additional comment changes. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: Confusing error message for REINDEX TABLE CONCURRENTLY
On Tue, Jun 04, 2019 at 11:26:44AM -0700, Ashwin Agrawal wrote: > The variable is used in else scope hence I moved it there. But yes its > removed completely for this scope. Thanks for updating the patch. It does its job by having one separate message for the concurrent and the non-concurrent cases as discussed. David, what do you think? Perhaps you would like to commit it yourself? -- Michael signature.asc Description: PGP signature
Re: Confusing error message for REINDEX TABLE CONCURRENTLY
On Mon, Jun 3, 2019 at 6:27 PM Michael Paquier wrote:
> On Mon, Jun 03, 2019 at 04:53:48PM -0700, Ashwin Agrawal wrote:
> > Please check if the attached patch addresses and satisfies all the points
> > discussed so far in this thread.
>
> It looks to be so, please see below for some comments.
>
> > +{
> > result = ReindexRelationConcurrently(heapOid, options);
> > +
> > +if (!result)
> > +ereport(NOTICE,
> > +(errmsg("table \"%s\" has no indexes that can be
> concurrently reindexed",
> > +relation->relname)));
>
> "concurrently" should be at the end of this string. I have had the
> exact same argument with Tom for 508300e.
>
Sure modified the same, find attached.
> > @@ -2630,7 +2638,6 @@ ReindexMultipleTables(const char *objectName,
> ReindexObjectType objectKind,
> > foreach(l, relids)
> > {
> > Oidrelid = lfirst_oid(l);
> > -boolresult;
> >
> > StartTransactionCommand();
> > /* functions in indexes may want a snapshot set */
> > @@ -2638,11 +2645,12 @@ ReindexMultipleTables(const char *objectName,
> ReindexObjectType objectKind,
> >
> > if (concurrent)
> > {
> > -result = ReindexRelationConcurrently(relid, options);
> > +ReindexRelationConcurrently(relid, options);
> > /* ReindexRelationConcurrently() does the verbose output */
>
> Indeed this variable is not used. So we could just get rid of it
> completely.
>
The variable is used in else scope hence I moved it there. But yes its
removed completely for this scope.
> +bool result;
> > result = reindex_relation(relid,
> >REINDEX_REL_PROCESS_TOAST |
> >REINDEX_REL_CHECK_CONSTRAINTS,
> > @@ -2656,7 +2664,6 @@ ReindexMultipleTables(const char *objectName,
> ReindexObjectType objectKind,
> >
> > PopActiveSnapshot();
> > }
>
> The table has been considered for reindexing even if nothing has been
> reindexed, so perhaps we'd want to keep this part as-is? We have the
> same level of reporting for a couple of releases for this part.
>
I don't understand the review comment. I functionally didn't change
anything in that part of code, just have result variable confined to that
scope of code.
> > -
> > CommitTransactionCommand();
>
> Useless noise diff.
>
Okay, removed it.
From 4486f9d114084e3b484be8d2ec0eb95b8f47 Mon Sep 17 00:00:00 2001
From: Ashwin Agrawal
Date: Mon, 3 Jun 2019 16:30:39 -0700
Subject: [PATCH v3] Avoid confusing error message for REINDEX.
---
src/backend/commands/indexcmds.c | 25 --
src/test/regress/expected/create_index.out | 10 -
2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 40ea629ffe7..a09ee059a30 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2438,17 +2438,25 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
RangeVarCallbackOwnsTable, NULL);
if (concurrent)
+ {
result = ReindexRelationConcurrently(heapOid, options);
+
+ if (!result)
+ ereport(NOTICE,
+ (errmsg("table \"%s\" has no indexes that can be reindexed concurrently",
+ relation->relname)));
+ }
else
+ {
result = reindex_relation(heapOid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
options);
-
- if (!result)
- ereport(NOTICE,
-(errmsg("table \"%s\" has no indexes",
- relation->relname)));
+ if (!result)
+ ereport(NOTICE,
+ (errmsg("table \"%s\" has no indexes to reindex",
+ relation->relname)));
+ }
return heapOid;
}
@@ -2630,7 +2638,6 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
foreach(l, relids)
{
Oid relid = lfirst_oid(l);
- bool result;
StartTransactionCommand();
/* functions in indexes may want a snapshot set */
@@ -2638,11 +2645,12 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
if (concurrent)
{
- result = ReindexRelationConcurrently(relid, options);
+ ReindexRelationConcurrently(relid, options);
/* ReindexRelationConcurrently() does the verbose output */
}
else
{
+ bool result;
result = reindex_relation(relid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
@@ -2676,6 +2684,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* The locks taken on parent tables and involved indexes are kept until the
* transaction is committed, at which point a session lock is taken on each
* relation. Both of these protect against concurrent schema changes.
+ *
+ * Returns true if any indexes have been rebuilt (including toast table's
+ * indexes when
Re: Confusing error message for REINDEX TABLE CONCURRENTLY
On Mon, Jun 03, 2019 at 04:53:48PM -0700, Ashwin Agrawal wrote:
> Please check if the attached patch addresses and satisfies all the points
> discussed so far in this thread.
It looks to be so, please see below for some comments.
> +{
> result = ReindexRelationConcurrently(heapOid, options);
> +
> +if (!result)
> +ereport(NOTICE,
> +(errmsg("table \"%s\" has no indexes that can be
> concurrently reindexed",
> +relation->relname)));
"concurrently" should be at the end of this string. I have had the
exact same argument with Tom for 508300e.
> @@ -2630,7 +2638,6 @@ ReindexMultipleTables(const char *objectName,
> ReindexObjectType objectKind,
> foreach(l, relids)
> {
> Oidrelid = lfirst_oid(l);
> -boolresult;
>
> StartTransactionCommand();
> /* functions in indexes may want a snapshot set */
> @@ -2638,11 +2645,12 @@ ReindexMultipleTables(const char *objectName,
> ReindexObjectType objectKind,
>
> if (concurrent)
> {
> -result = ReindexRelationConcurrently(relid, options);
> +ReindexRelationConcurrently(relid, options);
> /* ReindexRelationConcurrently() does the verbose output */
Indeed this variable is not used. So we could just get rid of it
completely.
> +bool result;
> result = reindex_relation(relid,
>REINDEX_REL_PROCESS_TOAST |
>REINDEX_REL_CHECK_CONSTRAINTS,
> @@ -2656,7 +2664,6 @@ ReindexMultipleTables(const char *objectName,
> ReindexObjectType objectKind,
>
> PopActiveSnapshot();
> }
The table has been considered for reindexing even if nothing has been
reindexed, so perhaps we'd want to keep this part as-is? We have the
same level of reporting for a couple of releases for this part.
> -
> CommitTransactionCommand();
Useless noise diff.
--
Michael
signature.asc
Description: PGP signature
Re: Confusing error message for REINDEX TABLE CONCURRENTLY
On Tue, May 28, 2019 at 12:05 PM David Rowley
wrote:
> On Tue, 28 May 2019 at 01:23, Ashwin Agrawal wrote:
> > I think we will need to separate out the NOTICE message for concurrent
> and regular case.
> >
> > For example this doesn't sound correct
> > WARNING: cannot reindex exclusion constraint index
> "public.circles_c_excl" concurrently, skipping
> > NOTICE: table "circles" has no indexes to reindex
> >
> > As no indexes can't be reindexed *concurrently* but there are still
> indexes which can be reindexed, invalid indexes I think fall in same
> category.
>
> Swap "can't" for "can" and, yeah. I think it would be good to make the
> error messages differ for these two cases. This would serve as a hint
> to the user that they might have better luck trying without the
> "concurrently" option.
>
Please check if the attached patch addresses and satisfies all the points
discussed so far in this thread.
Was thinking of adding explicit errhint for concurrent case NOTICE to
convey, either the table has no indexes or can only be reindexed without
CONCURRENTLY. But thought may be its obvious but feel free to add if would
be helpful.
From 2e9eceff07d9bdadef82a54c17f399263436bf9d Mon Sep 17 00:00:00 2001
From: Ashwin Agrawal
Date: Mon, 3 Jun 2019 16:30:39 -0700
Subject: [PATCH v2] Avoid confusing error message for REINDEX.
---
src/backend/commands/indexcmds.c | 26 +++---
src/test/regress/expected/create_index.out | 10 -
2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 40ea629ffe7..bab20028090 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2438,17 +2438,25 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
RangeVarCallbackOwnsTable, NULL);
if (concurrent)
+ {
result = ReindexRelationConcurrently(heapOid, options);
+
+ if (!result)
+ ereport(NOTICE,
+ (errmsg("table \"%s\" has no indexes that can be concurrently reindexed",
+ relation->relname)));
+ }
else
+ {
result = reindex_relation(heapOid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
options);
-
- if (!result)
- ereport(NOTICE,
-(errmsg("table \"%s\" has no indexes",
- relation->relname)));
+ if (!result)
+ ereport(NOTICE,
+ (errmsg("table \"%s\" has no indexes to reindex",
+ relation->relname)));
+ }
return heapOid;
}
@@ -2630,7 +2638,6 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
foreach(l, relids)
{
Oid relid = lfirst_oid(l);
- bool result;
StartTransactionCommand();
/* functions in indexes may want a snapshot set */
@@ -2638,11 +2645,12 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
if (concurrent)
{
- result = ReindexRelationConcurrently(relid, options);
+ ReindexRelationConcurrently(relid, options);
/* ReindexRelationConcurrently() does the verbose output */
}
else
{
+ bool result;
result = reindex_relation(relid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
@@ -2656,7 +2664,6 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
PopActiveSnapshot();
}
-
CommitTransactionCommand();
}
StartTransactionCommand();
@@ -2676,6 +2683,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* The locks taken on parent tables and involved indexes are kept until the
* transaction is committed, at which point a session lock is taken on each
* relation. Both of these protect against concurrent schema changes.
+ *
+ * Returns true if any indexes have been rebuilt (including toast table's
+ * indexes when relevant).
*/
static bool
ReindexRelationConcurrently(Oid relationOid, int options)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index c8bc6be0613..72a8fca48e4 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1944,9 +1944,9 @@ DROP TABLE reindex_verbose;
CREATE TABLE concur_reindex_tab (c1 int);
-- REINDEX
REINDEX TABLE concur_reindex_tab; -- notice
-NOTICE: table "concur_reindex_tab" has no indexes
+NOTICE: table "concur_reindex_tab" has no indexes to reindex
REINDEX TABLE CONCURRENTLY concur_reindex_tab; -- notice
-NOTICE: table "concur_reindex_tab" has no indexes
+NOTICE: table "concur_reindex_tab" has no indexes that can be concurrently reindexed
ALTER TABLE concur_reindex_tab ADD COLUMN c2 text; -- add toast index
-- Normal index with integer column
CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab(c1);
@@ -2043,10 +2043,10 @@ ERROR: REINDEX is not yet implemented for partitioned indexes
-- REINDEX is a no-op for partitioned tables
REINDEX TABLE concur_reindex_part_10;
WARNING: REINDEX of
Re: Confusing error message for REINDEX TABLE CONCURRENTLY
On Tue, 28 May 2019 at 01:23, Ashwin Agrawal wrote: > I think we will need to separate out the NOTICE message for concurrent and > regular case. > > For example this doesn't sound correct > WARNING: cannot reindex exclusion constraint index "public.circles_c_excl" > concurrently, skipping > NOTICE: table "circles" has no indexes to reindex > > As no indexes can't be reindexed *concurrently* but there are still indexes > which can be reindexed, invalid indexes I think fall in same category. Swap "can't" for "can" and, yeah. I think it would be good to make the error messages differ for these two cases. This would serve as a hint to the user that they might have better luck trying without the "concurrently" option. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: Confusing error message for REINDEX TABLE CONCURRENTLY
On Sun, May 26, 2019 at 6:43 PM Michael Paquier wrote: > As you mention for reindex_relation() no indexes <=> nothing to do, > still let's not rely on that. Instead of making the error message > specific to concurrent operations, I would suggest to change it to > "table foo has no indexes to reindex". What do you think about the > attached? > I think we will need to separate out the NOTICE message for concurrent and regular case. For example this doesn't sound correct WARNING: cannot reindex exclusion constraint index "public.circles_c_excl" concurrently, skipping NOTICE: table "circles" has no indexes to reindex As no indexes can't be reindexed *concurrently* but there are still indexes which can be reindexed, invalid indexes I think fall in same category.
Re: Confusing error message for REINDEX TABLE CONCURRENTLY
On Sat, May 25, 2019 at 02:42:59PM +1200, David Rowley wrote:
> Also, I think people probably will care more about the fact that
> nothing was done for that table rather than if the table happens to
> have no indexes. For the non-concurrently case, that just happened to
> be the same thing.
This is equally confusing for plain REINDEX as well, no? Taking your
previous example:
=# REINDEX TABLE listp;
WARNING: 0A000: REINDEX of partitioned tables is not yet implemented,
skipping "listp"
LOCATION: reindex_relation, index.c:3513
NOTICE: 0: table "listp" has no indexes
LOCATION: ReindexTable, indexcmds.c:2452
REINDEX
In this case the relation has partitioned indexes, not indexes, so
that's actually correct. Still it seems to me that some users could
get confused by the current wording.
For invalid indexes you would get that:
=# create table aa (a int);
CREATE TABLE
=# insert into aa values (1),(1);
INSERT 0 2
=# create unique index concurrently aai on aa(a);
ERROR: 23505: could not create unique index "aai"
DETAIL: Key (a)=(1) is duplicated.
SCHEMA NAME: public
TABLE NAME: aa
CONSTRAINT NAME: aai
LOCATION: comparetup_index_btree, tuplesort.c:405
=# reindex table concurrently aa;
WARNING: 0A000: cannot reindex invalid index "public.aai"
concurrently, skipping
LOCATION: ReindexRelationConcurrently, indexcmds.c:2772
NOTICE: 0: table "aa" has no indexes
LOCATION: ReindexTable, indexcmds.c:2452
REINDEX
As you mention for reindex_relation() no indexes <=> nothing to do,
still let's not rely on that. Instead of making the error message
specific to concurrent operations, I would suggest to change it to
"table foo has no indexes to reindex". What do you think about the
attached?
--
Michael
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 40ea629ffe..ac4a32953a 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2447,7 +2447,7 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
if (!result)
ereport(NOTICE,
-(errmsg("table \"%s\" has no indexes",
+(errmsg("table \"%s\" has no indexes to reindex",
relation->relname)));
return heapOid;
@@ -2676,6 +2676,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* The locks taken on parent tables and involved indexes are kept until the
* transaction is committed, at which point a session lock is taken on each
* relation. Both of these protect against concurrent schema changes.
+ *
+ * Returns true if any indexes have been rebuilt (including toast table's
+ * indexes when relevant).
*/
static bool
ReindexRelationConcurrently(Oid relationOid, int options)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index c8bc6be061..4b62c9045a 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1944,9 +1944,9 @@ DROP TABLE reindex_verbose;
CREATE TABLE concur_reindex_tab (c1 int);
-- REINDEX
REINDEX TABLE concur_reindex_tab; -- notice
-NOTICE: table "concur_reindex_tab" has no indexes
+NOTICE: table "concur_reindex_tab" has no indexes to reindex
REINDEX TABLE CONCURRENTLY concur_reindex_tab; -- notice
-NOTICE: table "concur_reindex_tab" has no indexes
+NOTICE: table "concur_reindex_tab" has no indexes to reindex
ALTER TABLE concur_reindex_tab ADD COLUMN c2 text; -- add toast index
-- Normal index with integer column
CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab(c1);
@@ -2043,10 +2043,10 @@ ERROR: REINDEX is not yet implemented for partitioned indexes
-- REINDEX is a no-op for partitioned tables
REINDEX TABLE concur_reindex_part_10;
WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10"
-NOTICE: table "concur_reindex_part_10" has no indexes
+NOTICE: table "concur_reindex_part_10" has no indexes to reindex
REINDEX TABLE CONCURRENTLY concur_reindex_part_10;
WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10"
-NOTICE: table "concur_reindex_part_10" has no indexes
+NOTICE: table "concur_reindex_part_10" has no indexes to reindex
SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index')
ORDER BY relid, level;
relid | parentrelid | level
@@ -2150,7 +2150,7 @@ DELETE FROM concur_reindex_tab4 WHERE c1 = 1;
-- The invalid index is not processed when running REINDEX TABLE.
REINDEX TABLE CONCURRENTLY concur_reindex_tab4;
WARNING: cannot reindex invalid index "public.concur_reindex_ind5" concurrently, skipping
-NOTICE: table "concur_reindex_tab4" has no indexes
+NOTICE: table "concur_reindex_tab4" has no indexes to reindex
\d concur_reindex_tab4
Table "public.concur_reindex_tab4"
Column | Type | Collation | Nullable | Default
signature.asc
Description: PGP signature
Re: Confusing error message for REINDEX TABLE CONCURRENTLY
On Sat, 25 May 2019 at 12:06, Ashwin Agrawal wrote: > Seems might be just emit the NOTICE "table xxx has no index", if really no > index for concurrent and non-concurrent case, make it consistent, less > confusing and leave it there. Attaching the patch to just do that. Thoughts? Would it not be better just to change the error message for the concurrent case so that it reads: "table \"%s\" has no indexes that can be concurrently reindexed" Otherwise, what you have now is still confusing for partitioned tables: postgres=# create table listp (a int primary key) partition by list(a); CREATE TABLE postgres=# REINDEX TABLE CONCURRENTLY listp; psql: WARNING: REINDEX of partitioned tables is not yet implemented, skipping "listp" psql: NOTICE: table "listp" has no indexes REINDEX Also, I think people probably will care more about the fact that nothing was done for that table rather than if the table happens to have no indexes. For the non-concurrently case, that just happened to be the same thing. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Confusing error message for REINDEX TABLE CONCURRENTLY
CREATE TABLE circles (c circle, EXCLUDE USING gist (c WITH &&));
REINDEX TABLE CONCURRENTLY circles;
WARNING: cannot reindex exclusion constraint index "public.circles_c_excl"
concurrently, skipping
NOTICE: table "circles" has no indexes
REINDEX
The message "table has no indexes" is confusing, as warning above it states
table has index, just was skipped by reindex.
So, currently for any reason (exclusion or invalid index) reindex table
concurrently skips reindex, it reports the table has no index. Looking at
the behavior of non-concurrent reindex, it emits the NOTICE only if table
really has no indexes (since it has no skip cases).
We need to see what really wish to communicate here, table has no indexes
or just that reindex was *not* performed or keep it simple and completely
avoid emitting anything. If we skip any indexes we anyways emit WARNING, so
that should be sufficient and nothing more needs to be conveyed.
In-case we wish to communicate no reindex was performed, what do we wish to
notify for empty tables?
Seems might be just emit the NOTICE "table xxx has no index", if really no
index for concurrent and non-concurrent case, make it consistent, less
confusing and leave it there. Attaching the patch to just do that. Thoughts?
From dc8119abe180cc14b0720c4de79495de4c62bf12 Mon Sep 17 00:00:00 2001
From: Ashwin Agrawal
Date: Fri, 24 May 2019 16:34:48 -0700
Subject: [PATCH v1] Avoid confusing error message for REINDEX CONCURRENTLY.
REINDEX CONCURRENTLY skips reindexing exclusion or invalid
indexes. But in such cases it incorrectly reported table has no
indexes. Make the behavior consistent with not concurrently REINDEX
command to emit the notice only if the table has no indexes.
---
src/backend/commands/indexcmds.c | 13 -
src/test/regress/expected/create_index.out | 1 -
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 40ea629ffe7..402440ebad0 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2630,7 +2630,6 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
foreach(l, relids)
{
Oid relid = lfirst_oid(l);
- bool result;
StartTransactionCommand();
/* functions in indexes may want a snapshot set */
@@ -2638,11 +2637,12 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
if (concurrent)
{
- result = ReindexRelationConcurrently(relid, options);
+ ReindexRelationConcurrently(relid, options);
/* ReindexRelationConcurrently() does the verbose output */
}
else
{
+ bool result;
result = reindex_relation(relid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
@@ -2656,7 +2656,6 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
PopActiveSnapshot();
}
-
CommitTransactionCommand();
}
StartTransactionCommand();
@@ -2693,6 +2692,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
char *relationName = NULL;
char *relationNamespace = NULL;
PGRUsage ru0;
+ bool rel_has_index = false;
/*
* Create a memory context that will survive forced transaction commits we
@@ -2759,6 +2759,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Relation indexRelation = index_open(cellOid,
ShareUpdateExclusiveLock);
+ rel_has_index = true;
if (!indexRelation->rd_index->indisvalid)
ereport(WARNING,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -2805,6 +2806,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Relation indexRelation = index_open(cellOid,
ShareUpdateExclusiveLock);
+ rel_has_index = true;
if (!indexRelation->rd_index->indisvalid)
ereport(WARNING,
(errcode(ERRCODE_INDEX_CORRUPTED),
@@ -2843,6 +2845,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex a system catalog concurrently")));
+rel_has_index = true;
/* Save the list of relation OIDs in private context */
oldcontext = MemoryContextSwitchTo(private_context);
@@ -2873,11 +2876,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
break;
}
- /* Definitely no indexes, so leave */
+ /* Definitely no indexes to rebuild, so leave */
if (indexIds == NIL)
{
PopActiveSnapshot();
- return false;
+ return rel_has_index;
}
Assert(heapRelationIds != NIL);
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index c8bc6be0613..9a9401c280e 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2150,7 +2150,6 @@ DELETE FROM concur_reindex_tab4 WHERE c1 = 1;
-- The invalid index is not processed when running REINDEX TABLE.
REINDEX TABLE CONCURRENTLY
