From: Tom Lane <tgl@sss.pgh.pa.us>
To: "Euler Taveira" <euler@eulerto.com>
cc: "Tender Wang" <tndrwang@gmail.com>, tharakan@gmail.com,
        pgsql-bugs@lists.postgresql.org,
        "Heikki Linnakangas" <hlinnaka@iki.fi>
Subject: Re: BUG #18866: Running pg_freespace() on views triggers an Abort
In-reply-to: <b6ea023a-703d-4afa-8561-9e867db1b5f0@app.fastmail.com>
References: <18866-d68926d0f1c72d44@postgresql.org>
 <CAHewXNk6uX0VBROmoMrnKs_AOzbAQo+i9pouUiYj01y50HZRSw@mail.gmail.com>
 <1280897.1742920635@sss.pgh.pa.us>
 <CAHewXNm8rkn9Vu-WhmMQMJS2hSqztbJO1cRv5-+Rp99+k43aJw@mail.gmail.com>
 <b6ea023a-703d-4afa-8561-9e867db1b5f0@app.fastmail.com>
Comments: In-reply-to "Euler Taveira" <euler@eulerto.com>
	message dated "Tue, 25 Mar 2025 22:55:52 -0300"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1618266.1743004859.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 26 Mar 2025 12:00:59 -0400
Message-ID: <1618267.1743004859@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1618267.1743004859%40sss.pgh.pa.us>
Precedence: bulk

"Euler Taveira" <euler@eulerto.com> writes:
> Your patch needs some adjustments. There is no need to include pg_class.=
h. I
> don't like the proposed error message. I prefer saying the relation cann=
ot be
> opened because that's what will happen if it reaches this code path.

I don't care for that proposal either: we just did open the relation ;-)

Looking at other places where we throw an error for !RELKIND_HAS_STORAGE:

rawpage.c:
                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                 errmsg("cannot get raw page from relation \"%s\"",

pgstatindex.c:

                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                 errmsg("cannot get page count of relation \"%s\"",

tid.c:
                errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                errmsg("cannot look at latest visible tid for relation \"%=
s.%s\"",

relcache.c:
        elog(ERROR, "relation \"%s\" does not have storage",
             RelationGetRelationName(relation));

So the previous proposal was evidently modeled on the first two of
these precedents.  Personally I prefer messages that say *why*
something failed, so I'd go with something more like "relation \"%s\"
does not have storage".  Use of errdetail_relkind_not_supported is
fine though.

			regards, tom lane