ruleutils vs. empty targetlists

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: ruleutils vs. empty targetlists
Date: 2013-12-03 23:37:23
Message-ID: 32067.1386113843@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thinking some more about bug #8648, it occurred to me that ruleutils.c
isn't exactly prepared for the case either:

regression=# create table nocols();
CREATE TABLE
regression=# create view vv1 as select exists (select * from nocols);
CREATE VIEW
regression=# \d+ vv1
View "public.vv1"
Column | Type | Modifiers | Storage | Description
--------+---------+-----------+---------+-------------
exists | boolean | | plain |
View definition:
SELECT (EXISTS ( SELECT
FROM nocols)) AS "exists";

which of course is illegal syntax. I thought at first that this could be
fixed trivially by emitting "*" if get_target_list found no columns.
However, that doesn't quite work; consider

create view vv2 as select exists (select nocols.* from nocols, somecols);

If we regurgitate this as "exists (select * from nocols, somecols)", it
wouldn't mean the same thing.

But on the third hand, at least in the context of an EXISTS() subquery,
it doesn't really matter because the tlist is irrelevant, at least as long
as it only contains Vars. So you could argue that emitting "*" is plenty
good enough even in the above example. I'm not certain that that applies
everywhere that a tlist could appear, though.

I experimented with code that would attempt to regurgitate "nocols.*"
in the above example; see attached draft patch. I don't like this patch
much: it's ugly, it'd be a pain to backport (because it's digging into
data structures that have changed repeatedly), and I'm not sure how much
I trust it anyway. So I'm leaning towards just doing

+ if (colno == 0)
+ appendStringInfoString(buf, " *");

at least till such time as somebody shows a case where this isn't good
enough.

Note that I wouldn't be thinking of this as something to back-patch
at all, except that if someone did have a view that looked like this,
they'd find that pg_dump output wouldn't restore. You could construct
scenarios where that could seem like a denial of service, particularly
if the DBA wasn't smart enough to figure out what was wrong with his
dump. (And who wants to deal with such a thing at 4AM anyway ...)

Comments?

regards, tom lane

Attachment Content-Type Size
empty-exists-patch-too-complicated.patch text/x-diff 1.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2013-12-03 23:51:29 Re: pgsql: Fix a couple of bugs in MultiXactId freezing
Previous Message Joshua D. Drake 2013-12-03 23:37:12 Re: Why we are going to have to go DirectIO