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)
>      {
>          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 */

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.