pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18866: Running pg_freespace() on views triggers an Abort
14+ messages / 7 participants
[nested] [flat]
* BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-25 14:26 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 14+ messages in thread
From: PG Bug reporting form @ 2025-03-25 14:26 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: tharakan@gmail.com
The following bug has been logged on the website:
Bug reference: 18866
Logged by: Robins Tharakan
Email address: tharakan@gmail.com
PostgreSQL version: Unsupported/Unknown
Operating system: Ubuntu
Description:
Hi,
Passing a view to pg_freespace() triggers an Abort on HEAD. This has been so
since the beginning (049ef3398d05c9dc8f48aa9a6d68440661cfeb87). Given that
this is just an assert, feel free to skip - but thought I'd bring it up, in
case this needs a review.
SQL / Backtrace / Error Log excerpt given below:
SQL
===
$ cat crashing.sql
CREATE EXTENSION pg_freespacemap;
SELECT pg_freespace('pg_roles', 0);
SQL Output
==========
$ psql -f crashing.sql
CREATE EXTENSION
psql:crashing.sql:2: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
psql:crashing.sql:2: error: connection to server was lost
Backtrace
=========
#5 0x00005b073976f7e0 in ExceptionalCondition (conditionName=0x5b07399bb930
"RelFileNumberIsValid(rlocator.relNumber)", fileName=0x5b07399bb90c
"smgr.c", lineNumber=228)
at assert.c:66
No locals.
#6 0x00005b0739556b62 in smgropen (rlocator=..., backend=-1) at
smgr.c:228
brlocator = {locator = {spcOid = 1101590320, dbOid = 23303,
relNumber = 1615569600}, backend = 31264}
reln = 0x5b0741ab40a8
found = 65
#7 0x00005b07395163cb in RelationGetSmgr (rel=0x7a2060b99bf8) at
../../../../src/include/utils/rel.h:579
No locals.
#8 0x00005b0739516dc2 in fsm_readbuf (rel=0x7a2060b99bf8, addr=...,
extend=false) at freespace.c:558
blkno = 2
buf = -998073056
reln = 0x0
#9 0x00005b0739516705 in GetRecordedFreeSpace (rel=0x7a2060b99bf8,
heapBlk=0) at freespace.c:254
addr = {level = 0, logpageno = 0}
slot = 0
buf = 23303
cat = 22 '\026'
#10 0x00007a2061ab630c in pg_freespace (fcinfo=0x5b0741b07928) at
pg_freespacemap.c:38
relid = 12000
blkno = 0
freespace = -17281
rel = 0x7a2060b99bf8
__func__ = "pg_freespace"
#11 0x00005b0739259dc4 in ExecInterpExpr (state=0x5b0741b077d0,
econtext=0x5b0741b074b8, isnull=0x0) at execExprInterp.c:1001
d = 100086724982736
fcinfo = 0x5b0741b07928
args = 0x5b0741b07948
op = 0x5b0741b07b50
resultslot = 0x5b0741b07708
innerslot = 0x0
outerslot = 0x0
scanslot = 0x0
oldslot = 0x0
newslot = 0x0
Surfacing Commit
================
Checking (62f36d6924c~0) - 62f36d6924 - fail
Checking (62f36d6924c~10) - 023fb51275 - fail
Checking (62f36d6924c~30) - e215166c9c - fail
.
.
.
Checking (62f36d6924c~3560) - b31ba5310b - fail
Checking (62f36d6924c~3565) - e5b8c4f68f - pass
Checking (62f36d6924c~3562) - 049ef3398d - fail
Checking (62f36d6924c~3563) - cd7f19da34 - pass
Surfacing commit is 049ef3398d05c9dc8f48aa9a6d68440661cfeb87
Postgres Error Logs
===================
TRAP: failed Assert("RelFileNumberIsValid(rlocator.relNumber)"), File:
"smgr.c", Line: 228, PID: 2372149
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(ExceptionalCondition+0xbb)[0x5b073976f7ad]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(smgropen+0x5e)[0x5b0739556b62]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x6b83cb)[0x5b07395163cb]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x6b8dc2)[0x5b0739516dc2]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(GetRecordedFreeSpace+0x50)[0x5b0739516705]
/home/smith/proj/blamepg/lib/postgresql/pg_freespacemap.so(pg_freespace+0xc4)[0x7a2061ab730c]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x3fbdc4)[0x5b0739259dc4]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(ExecInterpExprStillValid+0x5b)[0x5b073925c94a]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x461f27)[0x5b07392bff27]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x461fe5)[0x5b07392bffe5]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x462046)[0x5b07392c0046]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x462269)[0x5b07392c0269]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x418aba)[0x5b0739276aba]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x40aeaf)[0x5b0739268eaf]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x40df9e)[0x5b073926bf9e]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(standard_ExecutorRun+0x1cc)[0x5b07392696eb]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(ExecutorRun+0x54)[0x5b073926951c]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x707af2)[0x5b0739565af2]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(PortalRun+0x274)[0x5b0739565720]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x6ffc69)[0x5b073955dc69]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(PostgresMain+0xb8d)[0x5b0739563461]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x6fafe7)[0x5b0739558fe7]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(postmaster_child_launch+0x174)[0x5b073945cac4]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x605664)[0x5b0739463664]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(+0x602a7e)[0x5b0739460a7e]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(PostmasterMain+0x1546)[0x5b0739460374]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(main+0x38c)[0x5b07392ff7db]
/lib/x86_64-linux-gnu/libc.so.6(+0x2a1ca)[0x7a206042a1ca]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x8b)[0x7a206042a28b]
postgres: smith: smith postgres 127.0.0.1(51026)
SELECT(_start+0x25)[0x5b0738f43eb5]
2025-03-25 21:32:25.437 ACDT [2372118] LOG: client backend (PID 2372149)
was terminated by signal 6: Aborted
2025-03-25 21:32:25.437 ACDT [2372118] DETAIL: Failed process was running:
SELECT public.pg_freespace('pg_roles', 0);
2025-03-25 21:32:25.437 ACDT [2372118] LOG: terminating any other active
server processes
Thanks to SQLSmith / SQLReduce.
-
Robins
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-25 15:45 Tender Wang <tndrwang@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 14+ messages in thread
From: Tender Wang @ 2025-03-25 15:45 UTC (permalink / raw)
To: tharakan@gmail.com; pgsql-bugs@lists.postgresql.org; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>
PG Bug reporting form <noreply@postgresql.org> 于2025年3月25日周二 22:42写道：
> The following bug has been logged on the website:
>
> Bug reference: 18866
> Logged by: Robins Tharakan
> Email address: tharakan@gmail.com
> PostgreSQL version: Unsupported/Unknown
> Operating system: Ubuntu
> Description:
>
> Hi,
>
> Passing a view to pg_freespace() triggers an Abort on HEAD. This has been
> so
> since the beginning (049ef3398d05c9dc8f48aa9a6d68440661cfeb87). Given that
> this is just an assert, feel free to skip - but thought I'd bring it up, in
> case this needs a review.
>
> SQL / Backtrace / Error Log excerpt given below:
>
>
> SQL
> ===
> $ cat crashing.sql
> CREATE EXTENSION pg_freespacemap;
> SELECT pg_freespace('pg_roles', 0);
>
Yeah, it would crash if you input a foreign table, for example:
create extension postgres_fdw;
CREATE SERVER d FOREIGN DATA WRAPPER postgres_fdw;
CREATE FOREIGN TABLE f (g text) SERVER d;
SELECT pg_freespace('f', 0); -- will crash too
I think we can remove the Assert in smgropen().
Any thoughts?
--
Thanks,
Tender Wang
From 6b1f582b70164fdfe51675f6a450b91ca92f08be Mon Sep 17 00:00:00 2001
From: Tender Wang <tndrwang@gmail.com>
Date: Tue, 25 Mar 2025 23:40:53 +0800
Subject: [PATCH] Remove a wrong Assert().
---
src/backend/storage/smgr/smgr.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index af74f54b43..81ebbe67be 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -225,7 +225,6 @@ smgropen(RelFileLocator rlocator, ProcNumber backend)
SMgrRelation reln;
bool found;
- Assert(RelFileNumberIsValid(rlocator.relNumber));
HOLD_INTERRUPTS();
--
2.34.1
Attachments:
0001-Remove-a-wrong-Assert.patch (text/plain, 658B) download
From 6b1f582b70164fdfe51675f6a450b91ca92f08be Mon Sep 17 00:00:00 2001
From: Tender Wang <tndrwang@gmail.com>
Date: Tue, 25 Mar 2025 23:40:53 +0800
Subject: [PATCH] Remove a wrong Assert().
---
src/backend/storage/smgr/smgr.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index af74f54b43..81ebbe67be 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -225,7 +225,6 @@ smgropen(RelFileLocator rlocator, ProcNumber backend)
SMgrRelation reln;
bool found;
- Assert(RelFileNumberIsValid(rlocator.relNumber));
HOLD_INTERRUPTS();
--
2.34.1
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-25 16:37 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lane @ 2025-03-25 16:37 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
Tender Wang <tndrwang@gmail.com> writes:
> PG Bug reporting form <noreply@postgresql.org> 于2025年3月25日周二 22:42写道：
>> Passing a view to pg_freespace() triggers an Abort on HEAD.
> I think we can remove the Assert in smgropen().
> Any thoughts?
That seems like a "solution" that will allow other bugs. pg_freespace
should be fixed to verify RELKIND_HAS_STORAGE() before it tries to
access said storage.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-26 01:18 Tender Wang <tndrwang@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 14+ messages in thread
From: Tender Wang @ 2025-03-26 01:18 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
Tom Lane <tgl@sss.pgh.pa.us> 于2025年3月26日周三 00:37写道：
> Tender Wang <tndrwang@gmail.com> writes:
> > PG Bug reporting form <noreply@postgresql.org> 于2025年3月25日周二 22:42写道：
> >> Passing a view to pg_freespace() triggers an Abort on HEAD.
>
> > I think we can remove the Assert in smgropen().
> > Any thoughts?
>
> That seems like a "solution" that will allow other bugs. pg_freespace
> should be fixed to verify RELKIND_HAS_STORAGE() before it tries to
> access said storage.
>
Thanks for the advice. Please see the attached patch.
--
Thanks,
Tender Wang
From 36e34df5ff45456b9257251b78a73182ba0028f3 Mon Sep 17 00:00:00 2001
From: Tender Wang <tndrwang@gmail.com>
Date: Wed, 26 Mar 2025 09:14:08 +0800
Subject: [PATCH v2] Don't allow no storage relation to get FSM info.
---
contrib/pg_freespacemap/pg_freespacemap.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/contrib/pg_freespacemap/pg_freespacemap.c b/contrib/pg_freespacemap/pg_freespacemap.c
index 565167aaef..0c3a180250 100644
--- a/contrib/pg_freespacemap/pg_freespacemap.c
+++ b/contrib/pg_freespacemap/pg_freespacemap.c
@@ -9,8 +9,10 @@
#include "postgres.h"
#include "access/relation.h"
+#include "catalog/pg_class.h"
#include "fmgr.h"
#include "storage/freespace.h"
+#include "utils/rel.h"
PG_MODULE_MAGIC;
@@ -35,6 +37,13 @@ pg_freespace(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid block number")));
+ if ((!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot get page from relation \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+
freespace = GetRecordedFreeSpace(rel, blkno);
relation_close(rel, AccessShareLock);
--
2.34.1
Attachments:
v2-0001-Don-t-allow-no-storage-relation-to-get-FSM-info.patch (text/plain, 1.2K) download
From 36e34df5ff45456b9257251b78a73182ba0028f3 Mon Sep 17 00:00:00 2001
From: Tender Wang <tndrwang@gmail.com>
Date: Wed, 26 Mar 2025 09:14:08 +0800
Subject: [PATCH v2] Don't allow no storage relation to get FSM info.
---
contrib/pg_freespacemap/pg_freespacemap.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/contrib/pg_freespacemap/pg_freespacemap.c b/contrib/pg_freespacemap/pg_freespacemap.c
index 565167aaef..0c3a180250 100644
--- a/contrib/pg_freespacemap/pg_freespacemap.c
+++ b/contrib/pg_freespacemap/pg_freespacemap.c
@@ -9,8 +9,10 @@
#include "postgres.h"
#include "access/relation.h"
+#include "catalog/pg_class.h"
#include "fmgr.h"
#include "storage/freespace.h"
+#include "utils/rel.h"
PG_MODULE_MAGIC;
@@ -35,6 +37,13 @@ pg_freespace(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid block number")));
+ if ((!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot get page from relation \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+
freespace = GetRecordedFreeSpace(rel, blkno);
relation_close(rel, AccessShareLock);
--
2.34.1
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-26 01:55 Euler Taveira <euler@eulerto.com>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Euler Taveira @ 2025-03-26 01:55 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, "Heikki Linnakangas" <hlinnaka@iki.fi>
On Tue, Mar 25, 2025, at 10:18 PM, Tender Wang wrote:
> Thanks for the advice. Please see the attached patch.
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 cannot be
opened because that's what will happen if it reaches this code path. This is an
existing message so no need for translation. The output will be like:
postgres=# SELECT pg_freespace('f', 0);
ERROR: cannot open relation "f"
DETAIL: This operation is not supported for foreign tables.
postgres=# SELECT pg_freespace('pg_roles', 0);
ERROR: cannot open relation "pg_roles"
DETAIL: This operation is not supported for views.
postgres=# SELECT pg_freespace('pg_class', 0);
pg_freespace
--------------
7520
(1 row)
I'm attaching a patch that includes these modifications.
--
Euler Taveira
EDB https://www.enterprisedb.com/
Attachments:
fsm.patch (text/x-patch, 845B) download
diff --git a/contrib/pg_freespacemap/pg_freespacemap.c b/contrib/pg_freespacemap/pg_freespacemap.c
index 565167aaef0..49eea2823b9 100644
--- a/contrib/pg_freespacemap/pg_freespacemap.c
+++ b/contrib/pg_freespacemap/pg_freespacemap.c
@@ -11,6 +11,7 @@
#include "access/relation.h"
#include "fmgr.h"
#include "storage/freespace.h"
+#include "utils/rel.h"
PG_MODULE_MAGIC;
@@ -30,6 +31,13 @@ pg_freespace(PG_FUNCTION_ARGS)
rel = relation_open(relid, AccessShareLock);
+ if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot open relation \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+
if (blkno < 0 || blkno > MaxBlockNumber)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-26 16:00 Tom Lane <tgl@sss.pgh.pa.us>
parent: Euler Taveira <euler@eulerto.com>
1 sibling, 2 replies; 14+ messages in thread
From: Tom Lane @ 2025-03-26 16:00 UTC (permalink / raw)
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>
"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 cannot 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
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-26 16:51 Euler Taveira <euler@eulerto.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 14+ messages in thread
From: Euler Taveira @ 2025-03-26 16:51 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Tender Wang <tndrwang@gmail.com>; tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, "Heikki Linnakangas" <hlinnaka@iki.fi>
On Wed, Mar 26, 2025, at 1:00 PM, Tom Lane wrote:
> "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 cannot 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 ;-)
Fair point.
> 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.
I thought about saying "no storage" but don't know why was convinced by that
proposal. Your suggestion works for me.
--
Euler Taveira
EDB https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-26 17:33 Álvaro Herrera <alvherre@alvh.no-ip.org>
parent: Euler Taveira <euler@eulerto.com>
1 sibling, 1 reply; 14+ messages in thread
From: Álvaro Herrera @ 2025-03-26 17:33 UTC (permalink / raw)
To: Euler Taveira <euler@eulerto.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
On 2025-Mar-25, Euler Taveira wrote:
> On Tue, Mar 25, 2025, at 10:18 PM, Tender Wang wrote:
> > Thanks for the advice. Please see the attached patch.
>
> Your patch needs some adjustments. There is no need to include pg_class.h.
I think including pg_class.h (which is where RELKIND_HAS_STORAGE() is
defined and where errdetail_relkind_not_supported is declared) is better
than including utils/rel.h, which brings in a whole bunch of additional
stuff in addition to pg_class.h :-)
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end." (2nd Commandment for C programmers)
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-26 19:00 Euler Taveira <euler@eulerto.com>
parent: Álvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 1 reply; 14+ messages in thread
From: Euler Taveira @ 2025-03-26 19:00 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: Tender Wang <tndrwang@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, "Heikki Linnakangas" <hlinnaka@iki.fi>
On Wed, Mar 26, 2025, at 2:33 PM, Álvaro Herrera wrote:
> On 2025-Mar-25, Euler Taveira wrote:
>
> > On Tue, Mar 25, 2025, at 10:18 PM, Tender Wang wrote:
> > > Thanks for the advice. Please see the attached patch.
> >
> > Your patch needs some adjustments. There is no need to include pg_class.h.
>
> I think including pg_class.h (which is where RELKIND_HAS_STORAGE() is
> defined and where errdetail_relkind_not_supported is declared) is better
> than including utils/rel.h, which brings in a whole bunch of additional
> stuff in addition to pg_class.h :-)
It doesn't work because of RelationGetRelationName() that is defined into
utils/rel.h.
--
Euler Taveira
EDB https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-26 19:06 Alvaro Herrera <alvherre@alvh.no-ip.org>
parent: Euler Taveira <euler@eulerto.com>
0 siblings, 0 replies; 14+ messages in thread
From: Alvaro Herrera @ 2025-03-26 19:06 UTC (permalink / raw)
To: Euler Taveira <euler@eulerto.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
On 2025-Mar-26, Euler Taveira wrote:
> It doesn't work because of RelationGetRelationName() that is defined into
> utils/rel.h.
Doh, of course.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"I can't go to a restaurant and order food because I keep looking at the
fonts on the menu. Five minutes later I realize that it's also talking
about food" (Donald Knuth)
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-27 07:14 Tender Wang <tndrwang@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 14+ messages in thread
From: Tender Wang @ 2025-03-27 07:14 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Euler Taveira <euler@eulerto.com>; tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
Tom Lane <tgl@sss.pgh.pa.us> 于2025年3月27日周四 00:01写道：
> "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
> cannot 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.
Yes, I referred to the rawpage.c.
> 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.
>
PFA for the updated patch file.
--
Thanks,
Tender Wang
From 55c9f72af7dd9c4b853cb79460bbaa5808cff8db Mon Sep 17 00:00:00 2001
From: Tender Wang <tndrwang@gmail.com>
Date: Wed, 26 Mar 2025 09:14:08 +0800
Subject: [PATCH v3] Don't allow no storage relation to get FSM info.
---
contrib/pg_freespacemap/pg_freespacemap.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/contrib/pg_freespacemap/pg_freespacemap.c b/contrib/pg_freespacemap/pg_freespacemap.c
index 565167aaef..3e726a64b0 100644
--- a/contrib/pg_freespacemap/pg_freespacemap.c
+++ b/contrib/pg_freespacemap/pg_freespacemap.c
@@ -11,6 +11,7 @@
#include "access/relation.h"
#include "fmgr.h"
#include "storage/freespace.h"
+#include "utils/rel.h"
PG_MODULE_MAGIC;
@@ -35,6 +36,13 @@ pg_freespace(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid block number")));
+ if ((!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("relation \"%s\" does not have storage",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+
freespace = GetRecordedFreeSpace(rel, blkno);
relation_close(rel, AccessShareLock);
--
2.34.1
Attachments:
v3-0001-Don-t-allow-no-storage-relation-to-get-FSM-info.patch (text/plain, 1.2K) download
From 55c9f72af7dd9c4b853cb79460bbaa5808cff8db Mon Sep 17 00:00:00 2001
From: Tender Wang <tndrwang@gmail.com>
Date: Wed, 26 Mar 2025 09:14:08 +0800
Subject: [PATCH v3] Don't allow no storage relation to get FSM info.
---
contrib/pg_freespacemap/pg_freespacemap.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/contrib/pg_freespacemap/pg_freespacemap.c b/contrib/pg_freespacemap/pg_freespacemap.c
index 565167aaef..3e726a64b0 100644
--- a/contrib/pg_freespacemap/pg_freespacemap.c
+++ b/contrib/pg_freespacemap/pg_freespacemap.c
@@ -11,6 +11,7 @@
#include "access/relation.h"
#include "fmgr.h"
#include "storage/freespace.h"
+#include "utils/rel.h"
PG_MODULE_MAGIC;
@@ -35,6 +36,13 @@ pg_freespace(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid block number")));
+ if ((!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("relation \"%s\" does not have storage",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+
freespace = GetRecordedFreeSpace(rel, blkno);
relation_close(rel, AccessShareLock);
--
2.34.1
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-27 08:48 Richard Guo <guofenglinux@gmail.com>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Richard Guo @ 2025-03-27 08:48 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Euler Taveira <euler@eulerto.com>; tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
On Thu, Mar 27, 2025 at 4:14 PM Tender Wang <tndrwang@gmail.com> wrote:
> PFA for the updated patch file.
Not related to this issue but I wonder why blkno is verified after
the relation is opened. It can be verified beforehand, no?
Thanks
Richard
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-27 16:01 Tom Lane <tgl@sss.pgh.pa.us>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lane @ 2025-03-27 16:01 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Euler Taveira <euler@eulerto.com>; tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
Richard Guo <guofenglinux@gmail.com> writes:
> Not related to this issue but I wonder why blkno is verified after
> the relation is opened. It can be verified beforehand, no?
It makes sense to me to check the parameters left-to-right, so
I think verifying blkno after relation is fine. It's not like
there's value in optimizing the failure case.
This does, however, suggest that we ought to check the relkind
immediately after opening the rel, before the blkno check.
I'll adjust the patch that way and push.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18866: Running pg_freespace() on views triggers an Abort
@ 2025-03-28 01:59 Tender Wang <tndrwang@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 14+ messages in thread
From: Tender Wang @ 2025-03-28 01:59 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; Euler Taveira <euler@eulerto.com>; tharakan@gmail.com, pgsql-bugs@lists.postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
Tom Lane <tgl@sss.pgh.pa.us> 于2025年3月28日周五 00:01写道：
> Richard Guo <guofenglinux@gmail.com> writes:
> > Not related to this issue but I wonder why blkno is verified after
> > the relation is opened. It can be verified beforehand, no?
>
> It makes sense to me to check the parameters left-to-right, so
> I think verifying blkno after relation is fine. It's not like
> there's value in optimizing the failure case.
>
> This does, however, suggest that we ought to check the relkind
> immediately after opening the rel, before the blkno check.
> I'll adjust the patch that way and push.
>
Thanks for pushing.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2025-03-28 01:59 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-03-25 14:26 BUG #18866: Running pg_freespace() on views triggers an Abort PG Bug reporting form <noreply@postgresql.org>
2025-03-25 15:45 ` Tender Wang <tndrwang@gmail.com>
2025-03-25 16:37 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-03-26 01:18 ` Tender Wang <tndrwang@gmail.com>
2025-03-26 01:55 ` Euler Taveira <euler@eulerto.com>
2025-03-26 16:00 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-03-26 16:51 ` Euler Taveira <euler@eulerto.com>
2025-03-27 07:14 ` Tender Wang <tndrwang@gmail.com>
2025-03-27 08:48 ` Richard Guo <guofenglinux@gmail.com>
2025-03-27 16:01 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-03-28 01:59 ` Tender Wang <tndrwang@gmail.com>
2025-03-26 17:33 ` Álvaro Herrera <alvherre@alvh.no-ip.org>
2025-03-26 19:00 ` Euler Taveira <euler@eulerto.com>
2025-03-26 19:06 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox