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.
Thanks,
Amit
>From 2db5f57f42c52236459ffefdd45cb3b7fe82ff56 Mon Sep 17 00:00:00 2001
From: amit <[email protected]>
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 =
makeRangeVarFromNameList(textToQualifiedNameList(relname));
- 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",
+
RelationGetRelationName(rel)),
+ 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",
+
RelationGetRelationName(rel))));
/*
* check permissions: must have SELECT on table or be in
--
2.11.0
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
