API change advice: Passing plan invalidation info from the rewriter into the planner?

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndQuadrant(dot)com>, Andres Freund <andres(at)2ndquadrant(dot)com>, Greg Smith <greg(at)2ndQuadrant(dot)com>, Yeb Havinga <yeb(dot)havinga(at)portavita(dot)nl>
Subject: API change advice: Passing plan invalidation info from the rewriter into the planner?
Date: 2014-03-05 05:55:11
Message-ID: 5316BC3F.4040908@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all

One of the remaining issues with row security is how to pass plan
invalidation information generated in the rewriter back into the planner.

With row security, it's necessary to set a field in PlannerGlobal,
tracking the user ID of the user the query was planned for if row
security was applied. It is also necessary to add a PlanInvalItem for
the user ID.

Currently the rewriter has no way to pass this information to the
planner. QueryRewrite returns just a Query*.

We use Query structs throughout the rewriter and planner; it doesn't
make sense to add a List* field for PlanInvalItem nodes and an Oid field
for the user ID to the Query node when it's only ever going to get used
for the top level Query node returned by the rewriter, and only for long
enough to copy the data into PlannerGlobal.

The alternative seems to be changing the return type of QueryRewrite,
introducing a new node type, say:

struct RewriteResult {
Query *productQuery;
Oid planUserId;
List* planInvalItems;
}

This seems cleaner, and more extensible, but it means changing a fair
bit of API, including:

pg_plan_query
planner
standard_planner
planner_hook_type
QueryRewrite

and probably the plan cache infrastructure too. So it'd be fairly
invasive, and I know that creates concerns about backpatching and
extensions.

I can't just polymorphically subclass Query as some kind of "TopQuery" -
no true polymorphism in C, would need a new NodeType for it, and then
need to teach everything that knows about T_Query about T_TopQuery too.
So that won't work.

So, I'm looking for advice before I embark on this change. I need _some_
way to pass invalidation information from the rewriter into the planner
when it's collected by row security code during rewriting.

Any advice/comments?

I'm inclined to bite the bullet and make the API change. It'll be a
pain, but I can see future uses for passing global info out of the
rewriter rather than shoving it into per-Query structures. I'd define a
RewriteResult and pass that down into all the rewriter internal
functions, then return the outer query wrapped in it.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2014-03-05 06:28:41 Re: UNION ALL on partitioned tables won't use indices.
Previous Message Amit Langote 2014-03-05 03:07:30 Re: The behavior of CheckRequiredParameterValues()