From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: wrong relkind error messages
In-reply-to: <dc35a398-37d0-75ce-07ea-1dd71d98f8ec@2ndquadrant.com>
References: <dc35a398-37d0-75ce-07ea-1dd71d98f8ec@2ndquadrant.com>
Comments: In-reply-to Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
	message dated "Mon, 13 Apr 2020 15:54:48 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <988.1586790795.1@sss.pgh.pa.us>
Date: Mon, 13 Apr 2020 11:13:15 -0400
Message-ID: <989.1586790795@sss.pgh.pa.us>
Precedence: bulk

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> I'm not a fan of error messages like
>      relation "%s" is not a table, foreign table, or materialized view

Agreed, they're not great.

> For example:

> -ERROR:  relation "ti" is not a table, foreign table, or materialized view
> +ERROR:  cannot define statistics for relation "ti"
> +DETAIL:  "ti" is an index.

I see where you'e going, and it seems like a generally-better idea,
but I feel like this phrasing is omitting some critical background
information that users don't necessarily have.  At the very least
it's not stating clearly that the failure is *because* ti is an
index.  More generally, the whole concept that statistics can only
be defined for certain kinds of relations has disappeared from view.
I fear that users who're less deeply into Postgres hacking than we
are might not have that concept at all, or at least it might not
come to mind immediately when they get this message.

Fixing this while avoiding your concern about proliferation of messages
seems a bit difficult though.  The best I can do after a couple minutes'
thought is

ERROR:  cannot define statistics for relation "ti"
DETAIL:  "ti" is an index, and this operation is not supported for that
kind of relation.

which seems a little long and awkward.  Another idea is

ERROR:  cannot define statistics for relation "ti"
DETAIL:  This operation is not supported for indexes.

which still leaves implicit that "ti" is an index, but probably that's
something the user can figure out.

Maybe someone else can do better?

			regards, tom lane