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

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

On 03/06/2014 02:58 AM, Tom Lane wrote:
> Craig Ringer <craig(at)hobby(dot)2ndquadrant(dot)com> writes:
>> 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.
>
> TBH I'd just add a user OID field in struct Query and not hack up a bunch
> of existing function APIs. It's not much worse than the existing
> constraintDeps field.

If you're happy with that, I certainly won't complain. It's much simpler
and less intrusive.

I should be able to post an update using this later today.

> The PlanInvalItem could perfectly well be generated by the planner,
> no, if it has the user OID? But I'm not real sure why you need it.
> I don't see the reason for an invalidation triggered by user ID.
> What exactly about the *user*, and not something else, would trigger
> plan invalidation?

It's only that the plan depends on the user ID. There's no point keeping
the plan around if the user no longer exists.

You're quite right that this can be done in the planner when a
dependency on the user ID is found, though. So there's no need to pass a
PlanInvalItem down, which is a lot nicer.

> What we do need is a notion that a plan cache entry might only be
> valid for a specific calling user ID; but that's a matter for cache
> entry lookup not for subsequent invalidation.

Yes, that would be good, but is IMO more of a separate optimization. I'm
currently using KaiGai's code to invalidate and re-plan when a user ID
change is detected.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2014-03-07 06:24:56 syslog_ident mentioned as syslog_identify in the docs
Previous Message Tom Lane 2014-03-07 05:17:47 Re: Securing "make check" (CVE-2014-0067)