Re: [COMMITTERS] pgsql: Add regression test for bug fixed by recent refactoring.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Kevin Grittner <kgrittn(at)postgresql(dot)org>, pgsql-committers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add regression test for bug fixed by recent refactoring.
Date: 2013-05-01 16:20:25
Message-ID: 5859.1367425225@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

I wrote:
> Of course the select from vv2 should fail as well, but it doesn't,
> because vv2 is nowhere to be seen in the rangetable passed to the
> executor. I think the planner is probably trashing the rangetable
> once it realizes that the query is completely dummy; but this is wrong
> because we need to leave view rangetable entries behind for permissions
> checks, even when they're unreferenced in the finished plan.

I found the problem. set_subquery_pathlist does this when the subquery
is demonstrably empty (ie "select something where false"):

/*
* It's possible that constraint exclusion proved the subquery empty. If
* so, it's convenient to turn it back into a dummy path so that we will
* recognize appropriate optimizations at this level.
*/
if (is_dummy_plan(rel->subplan))
{
set_dummy_rel_pathlist(rel);
return;
}

The trouble is that, with no SubqueryScan path, setrefs.c doesn't
realize that it needs to pull up the subquery's rangetable into the main
query. We could just delete the above chunk of code, but that would
result in worse planning choices in some complicated queries.
What I'm planning to do instead is teach createplan.c to compensate for
this by wrapping the dummy Result plan in a SubqueryScan, thus producing
the same plan tree as if set_subquery_pathlist had not done the above.
This is a tad Rube Goldbergian, perhaps, because after setrefs.c has
done its thing it will conclude that the SubqueryScan is pointless and
strip it off again :-). (So we end up with the same finished plan tree
as now, only it will have all the rangetable entries it should have for
permissions checking purposes.) However, the alternative seems to be
to devise some entirely new method for setrefs.c to figure out which
subqueries' rangetables still need to be pulled up into the main query
and which don't. That would be a much more invasive patch, without a
lot of redeeming social value AFAICS.

regards, tom lane

Attachment Content-Type Size
draft-fix-dummy-subquery.patch text/x-patch 4.4 KB

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2013-05-01 22:27:15 pgsql: Fix permission tests for views/tables proven empty by constraint
Previous Message Simon Riggs 2013-05-01 16:10:30 Re: [COMMITTERS] pgsql: Make fast promotion the default promotion mode.

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2013-05-01 16:27:55 Documentation epub format
Previous Message David Fetter 2013-05-01 16:14:46 Re: Re: [BUGS] BUG #8128: pg_dump (>= 9.1) failed while dumping a scheme named "old" from PostgreSQL 8.4