On 2017/06/13 22:53, Peter Eisentraut wrote:
> On 6/12/17 21:10, Amit Langote wrote:
>> On 2017/06/13 0:29, Peter Eisentraut wrote:
>>> On 4/24/17 21:22, Amit Langote wrote:
>>>>>> create extension pgrowlocks;
>>>>>> create view one as select 1;
>>>>>> select pgrowlocks('one');
>>>>>> -- ERROR:  could not open file "base/68730/68748": No such file or 
>>>>>> directory
>>>>>> With the attached patch:
>>>>>> select pgrowlocks('one');
>>>>>> ERROR:  "one" is not a table, index, materialized view, sequence, or 
>>>>>> TOAST
>>>>>> table
>> FWIW, patch seems simple enough to be committed into 10, unless I am
>> missing something.
>> Rebased one attached.
> According to CheckValidRowMarkRel() in execMain.c, we don't allow row
> locking in sequences, toast tables, and materialized views.  This is not
> quite the same as what your patch wants to do.

That's a good point.  Perhaps, running pgrowlocks() should only be
supported for relation kinds that allow locking rows.  That would be
logically consistent.

> I suppose we could still
> allow reading the relation, and it won't ever show anything
> interesting.  What do you think?

I was just thinking about fixing heap_beginscan() resulting in "could not
open file..." error (which is not very helpful) when pgrowlocks() is
passed the name of a file-less relation.

How about the attached patch that teaches pgrowlocks() to error out unless
a meaningful result can be returned?  With the patch, it will issue a "%s
is not a table" message if it is not a RELKIND_RELATION, except a
different message will be issued for partitioned tables (for they are
tables).  You might ask why I changed heap_openrv() to relation_openrv()?
That's because heap_openrv() results in "%s is an index" message if passed
an index relation, but perhaps it's better to be consistent about
outputting the same "%s is not a table" message even in all cases
including for indexes.

>From 2db5f57f42c52236459ffefdd45cb3b7fe82ff56 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 14 Jun 2017 14:22:36 +0900
Subject: [PATCH] Teach pgrowlocks to check relkind before scanning

 contrib/pgrowlocks/pgrowlocks.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c
index 00e2015c5c..5d52f9ec9a 100644
--- a/contrib/pgrowlocks/pgrowlocks.c
+++ b/contrib/pgrowlocks/pgrowlocks.c
@@ -97,7 +97,19 @@ pgrowlocks(PG_FUNCTION_ARGS)
                relname = PG_GETARG_TEXT_PP(0);
                relrv = 
-               rel = heap_openrv(relrv, AccessShareLock);
+               rel = relation_openrv(relrv, AccessShareLock);
+               if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                        errmsg("\"%s\" is a partitioned table",
+                                        errdetail("Partitioned tables do not 
contain rows.")));
+               else if (rel->rd_rel->relkind != RELKIND_RELATION)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                        errmsg("\"%s\" is not a table",
                 * check permissions: must have SELECT on table or be in

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to