Re: [PATCHES] Updatable views

--On Donnerstag, August 31, 2006 11:10:47 -0400 Tom Lane
<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> The problem is not with the view condition. Consider
>
> CREATE TABLE data (id serial primary key, ...);
>
> CREATE VIEW only_new_data AS SELECT * FROM data WHERE id > 12345
> WITH CHECK OPTION;
>
> INSERT INTO only_new_data VALUES(nextval('data_id_seq'), ...);
>
> The proposed implementation will execute nextval twice (bad), and will
> apply the WITH CHECK OPTION test to the value that isn't the one stored
> (much worse). It doesn't help if the id is defaulted.

*scratches head*....i don't see a shortcoming solution for this in my
current implementation,
indeed. I admit that this is a serious containment of updatable views in
its current
incarnation (at least for the check option).

*thinking*
I would like to try to grab your idea to push down the CHECK OPTION logic
down to the executor as a (table/view?) constraint. Would that be an idea
worth to consider and,
most important, is this doable? I don't have that much experience in the
executor, so i fear
this isn't something that will be done within a week or so.....:(

--
Thanks

Bernd

Bernd Helmle <bernd(dot)helmle(at)oopsware(dot)de> writes:
> I would like to try to grab your idea to push down the CHECK OPTION logic
> down to the executor as a (table/view?) constraint. Would that be an idea
> worth to consider and,
> most important, is this doable? I don't have that much experience in the
> executor, so i fear
> this isn't something that will be done within a week or so.....:(

You're certainly welcome to work on it --- I don't have time at the
moment. But I agree there's little chance of getting it done in time
for 8.2.

I have not read the spec's definition of WITH CHECK OPTION lately, so
there may be something fundamentally wrong in what I'm about to say,
but the way I'm envisioning this working is that W.C.O. is embodied as
a check constraint (or something pretty similar --- a pg_constraint
entry certainly) attached to the view. The rewriter would then be
responsible for collecting all the check options that need to be
enforced for a given rewritten query. This'd require some rework of
the rewriter/planner/executor API: right now the executor is solely
responsible for collecting check constraints to apply in an updating
query, and we'd want to change that. My thought is we might as well
move the collection responsibility over to the rewriter 100%: the
rewriter decorates a Query with the list of constraint expressions
to apply, and the executor just checks what it's told to. The planner
probably need not do much with the constraint expressions beyond what
it normally does with, say, targetlist expressions.

Some thoughts:

* It's too early in the morning for me to be clear about the difference
between CASCADED and LOCAL CHECK OPTION --- I think that this would
merely alter the set of check constraints collected for a particular
query, but if there's something more fundamental there, this scheme
might not work at all. So look into that first.

* The reason we currently collect constraints at executor start is that
ALTER TABLE ADD CONSTRAINT has no way to invalidate cached plans, so
it's unsafe to store lists of constraints in plans. So this scheme
*requires* that we have a plan invalidation mechanism in place (at least
before we release, maybe not before the patch can go in). This doesn't
bother me because I intend anyway to see to it that there's plan inval
in 8.3.

* With check constraints now passing through the planner, it'd become
trivial to allow subqueries in check constraints. Just sayin'.

* We'd probably eliminate the idea of storing constraints in TupleDescs,
which would be a good simplification anyway (eg, eliminate the bogus
distinction between CreateTupleDescCopy and CreateTupleDescCopyConstr).
OTOH that might make it harder to allow rowtypes to have associated
constraints? Needs thought.

regards, tom lane

[Quick note: sorry if you received this mail multiple times,
i've moved to a new workstation and my MUA gots hosed up
with its identities (all of them has the same adress, suddenly)
and I recognized that too late.....i'm sorry]

--On Freitag, September 01, 2006 10:03:42 -0400 Tom Lane
<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Bernd Helmle <bernd(dot)helmle(at)oopsware(dot)de> writes:

>
> You're certainly welcome to work on it --- I don't have time at the
> moment. But I agree there's little chance of getting it done in time
> for 8.2.
>

What we can do is to restrict view updates that involves a volatile function
completely. As soon as the rewriter wants to apply an implicit system rule
to a current query which holds volatile functions, we could treat this as
an error.
However, i haven't looked into that right now how doable that would be, but
it seems correct in terms of data reliability (treat it as "volatile in
view update is
evil" ;)

Maybe it's worth to look how other database systems solve this problem.

> I have not read the spec's definition of WITH CHECK OPTION lately, so
> there may be something fundamentally wrong in what I'm about to say,
> but the way I'm envisioning this working is that W.C.O. is embodied as
> a check constraint (or something pretty similar --- a pg_constraint
> entry certainly) attached to the view. The rewriter would then be
> responsible for collecting all the check options that need to be
> enforced for a given rewritten query. This'd require some rework of
> the rewriter/planner/executor API: right now the executor is solely
> responsible for collecting check constraints to apply in an updating
> query, and we'd want to change that. My thought is we might as well
> move the collection responsibility over to the rewriter 100%: the
> rewriter decorates a Query with the list of constraint expressions
> to apply, and the executor just checks what it's told to. The planner
> probably need not do much with the constraint expressions beyond what
> it normally does with, say, targetlist expressions.

In order you want to do a CASCADED CHECK OPTION,
you need to collect all expressions out of underlying relations and rewrite
them to match
the table you are selecting...that looks like a very expensive operation.

>
> Some thoughts:
>
> * It's too early in the morning for me to be clear about the difference
> between CASCADED and LOCAL CHECK OPTION --- I think that this would
> merely alter the set of check constraints collected for a particular
> query, but if there's something more fundamental there, this scheme
> might not work at all. So look into that first.
>

LOCAL checks the data to be updated against its own view WHERE condition
only, where
CASCADED involves all WHERE conditions of all underlying views. That said,
it's
necessary to grep out all WHERE conditions of all relations involved in an
update operation and apply them to the current query as a constraint
expression. The
current implementation passes this recursively via a conditional rule
through the
rewriter. It looked to me as an attractive implementation, but it has this
annoying
multiple evaluation side effects....:(

> * The reason we currently collect constraints at executor start is that
> ALTER TABLE ADD CONSTRAINT has no way to invalidate cached plans, so
> it's unsafe to store lists of constraints in plans. So this scheme
> *requires* that we have a plan invalidation mechanism in place (at least
> before we release, maybe not before the patch can go in). This doesn't
> bother me because I intend anyway to see to it that there's plan inval
> in 8.3.
>

So we need to stall this idea unless we have something workable in this
area. So
what's the plan for 8.2? Should we reject updatable views completely or
is there some interest to apply this without CHECK OPTION? Some basic
functionality
could be simulated with table constraints, however, it's not what users out
there
would expect....

> * With check constraints now passing through the planner, it'd become
> trivial to allow subqueries in check constraints. Just sayin'.
>

That would be a nice feature, indeed ;)

> * We'd probably eliminate the idea of storing constraints in TupleDescs,
> which would be a good simplification anyway (eg, eliminate the bogus
> distinction between CreateTupleDescCopy and CreateTupleDescCopyConstr).
> OTOH that might make it harder to allow rowtypes to have associated
> constraints? Needs thought.
>
> regards, tom lane

--
Thanks

Bernd

Bernd Helmle wrote:

> <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> >* It's too early in the morning for me to be clear about the difference
> >between CASCADED and LOCAL CHECK OPTION --- I think that this would
> >merely alter the set of check constraints collected for a particular
> >query, but if there's something more fundamental there, this scheme
> >might not work at all. So look into that first.
>
> LOCAL checks the data to be updated against its own view WHERE
> condition only, where CASCADED involves all WHERE conditions of all
> underlying views.

I don't understand this part very well. Say if you have a view WITH
CHECK OPTION whose condition is "foo > 5", and then define a view WITH
LOCAL CHECK OPTION on top of that, whose condition is "bar > 5". Does
the local check option on the second view that I can insert a row with
foo=4, bar=6? That doesn't violate the condition of bar > 5, so it
seems fine to me. But it also seems quite idiotic because it violated
the original foo>5 condition.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Bernd Helmle <mailings(at)oopsware(dot)de> writes:
> What we can do is to restrict view updates that involves a volatile function
> completely. As soon as the rewriter wants to apply an implicit system rule
> to a current query which holds volatile functions, we could treat this as
> an error.

So in other words, views on serial columns don't work? I don't think
that's going to be acceptable.

> In order you want to do a CASCADED CHECK OPTION,
> you need to collect all expressions out of underlying relations and rewrite
> them to match
> the table you are selecting...that looks like a very expensive operation.

Not really worse than what the rewriter is doing already --- in fact,
I think it's isomorphic to what would happen to the rule qual
expressions in your existing patch.

> So we need to stall this idea unless we have something workable in
> this area. So what's the plan for 8.2? Should we reject updatable
> views completely or is there some interest to apply this without CHECK
> OPTION?

I'm about to propose that we should try to go beta next week (see
forthcoming message). If you can strip down your patch to avoid the
multi-eval problems in the next couple of days, I'm still willing to
consider it, but at the moment I'm assuming that it needs to be held
for 8.3.

regards, tom lane

On 9/1/06, Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:
> Bernd Helmle wrote:
>
> > <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > >* It's too early in the morning for me to be clear about the difference
> > >between CASCADED and LOCAL CHECK OPTION --- I think that this would
> > >merely alter the set of check constraints collected for a particular
> > >query, but if there's something more fundamental there, this scheme
> > >might not work at all. So look into that first.
> >
> > LOCAL checks the data to be updated against its own view WHERE
> > condition only, where CASCADED involves all WHERE conditions of all
> > underlying views.
>
> I don't understand this part very well. Say if you have a view WITH
> CHECK OPTION whose condition is "foo > 5", and then define a view WITH
> LOCAL CHECK OPTION on top of that, whose condition is "bar > 5". Does
> the local check option on the second view that I can insert a row with
> foo=4, bar=6? That doesn't violate the condition of bar > 5, so it

yes. or at least that's the way i read that...

> seems fine to me. But it also seems quite idiotic because it violated
> the original foo>5 condition.
>

and that means that without the CHECK OPTION constraint you can insert
anything into the base table no matter what the WHERE condition in the
view is...
again, this is the way informix implements it...

ahhh, the great members of the SQL COMITTEE...

--
regards,
Jaime Casanova

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook

--On Freitag, September 01, 2006 11:34:49 -0400 Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:

> I don't understand this part very well. Say if you have a view WITH
> CHECK OPTION whose condition is "foo > 5", and then define a view WITH
> LOCAL CHECK OPTION on top of that, whose condition is "bar > 5". Does
> the local check option on the second view that I can insert a row with
> foo=4, bar=6? That doesn't violate the condition of bar > 5, so it
> seems fine to me. But it also seems quite idiotic because it violated
> the original foo>5 condition.

That's exactly what i'm reading out there, too. If such a view definition
is useful or not
depends on its use case. Correct me if i'm wrong....

--
Thanks

Bernd

--On Freitag, September 01, 2006 11:41:16 -0400 Tom Lane
<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

>
> So in other words, views on serial columns don't work? I don't think
> that's going to be acceptable.
>

They work in such a case that someone isn't allowed to put a volatile
function in an update query....

>
> Not really worse than what the rewriter is doing already --- in fact,
> I think it's isomorphic to what would happen to the rule qual
> expressions in your existing patch.
>

Currently you don't have to rewrite the rule conditions itself every
time you apply them to the query tree since they are stored in pg_rewrite
matching all various (reversed) varattno's, resno's and whatever.
If i understand correctly you need to do that with constraints every time
you fire an update query on a view for each underlying relation....

>
> I'm about to propose that we should try to go beta next week (see
> forthcoming message). If you can strip down your patch to avoid the
> multi-eval problems in the next couple of days, I'm still willing to

Depends on how many days "couple of days" are.....if you mean the next
three days
then definitely not, since i'm afk for the whole upcoming weekend. Bad
timing, but
it's not deferrable.... :(

> consider it, but at the moment I'm assuming that it needs to be held
> for 8.3.

Well, i'll see what i can do next week....it's a little bit disappointing
that these problems
raises so late, but that's no one's fault since there are many side effects
of the rewriting
system involved....

Many thanks for your comments.

Bernd

This has been saved for the 8.3 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Bernd Helmle wrote:
> --On Freitag, September 01, 2006 11:41:16 -0400 Tom Lane
> <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> >
> > So in other words, views on serial columns don't work? I don't think
> > that's going to be acceptable.
> >
>
> They work in such a case that someone isn't allowed to put a volatile
> function in an update query....
>
> >
> > Not really worse than what the rewriter is doing already --- in fact,
> > I think it's isomorphic to what would happen to the rule qual
> > expressions in your existing patch.
> >
>
> Currently you don't have to rewrite the rule conditions itself every
> time you apply them to the query tree since they are stored in pg_rewrite
> matching all various (reversed) varattno's, resno's and whatever.
> If i understand correctly you need to do that with constraints every time
> you fire an update query on a view for each underlying relation....
>
> >
> > I'm about to propose that we should try to go beta next week (see
> > forthcoming message). If you can strip down your patch to avoid the
> > multi-eval problems in the next couple of days, I'm still willing to
>
> Depends on how many days "couple of days" are.....if you mean the next
> three days
> then definitely not, since i'm afk for the whole upcoming weekend. Bad
> timing, but
> it's not deferrable.... :(
>
> > consider it, but at the moment I'm assuming that it needs to be held
> > for 8.3.
>
> Well, i'll see what i can do next week....it's a little bit disappointing
> that these problems
> raises so late, but that's no one's fault since there are many side effects
> of the rewriting
> system involved....
>
> Many thanks for your comments.
>
> Bernd
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Here is the more recent email I have on this feature work.

---------------------------------------------------------------------------

Bernd Helmle wrote:
> --On Freitag, September 01, 2006 11:41:16 -0400 Tom Lane
> <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> >
> > So in other words, views on serial columns don't work? I don't think
> > that's going to be acceptable.
> >
>
> They work in such a case that someone isn't allowed to put a volatile
> function in an update query....
>
> >
> > Not really worse than what the rewriter is doing already --- in fact,
> > I think it's isomorphic to what would happen to the rule qual
> > expressions in your existing patch.
> >
>
> Currently you don't have to rewrite the rule conditions itself every
> time you apply them to the query tree since they are stored in pg_rewrite
> matching all various (reversed) varattno's, resno's and whatever.
> If i understand correctly you need to do that with constraints every time
> you fire an update query on a view for each underlying relation....
>
> >
> > I'm about to propose that we should try to go beta next week (see
> > forthcoming message). If you can strip down your patch to avoid the
> > multi-eval problems in the next couple of days, I'm still willing to
>
> Depends on how many days "couple of days" are.....if you mean the next
> three days
> then definitely not, since i'm afk for the whole upcoming weekend. Bad
> timing, but
> it's not deferrable.... :(
>
> > consider it, but at the moment I'm assuming that it needs to be held
> > for 8.3.
>
> Well, i'll see what i can do next week....it's a little bit disappointing
> that these problems
> raises so late, but that's no one's fault since there are many side effects
> of the rewriting
> system involved....
>
> Many thanks for your comments.
>
> Bernd
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Where are on updatable views?

---------------------------------------------------------------------------

Bernd Helmle wrote:
> --On Freitag, September 01, 2006 11:41:16 -0400 Tom Lane
> <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> >
> > So in other words, views on serial columns don't work? I don't think
> > that's going to be acceptable.
> >
>
> They work in such a case that someone isn't allowed to put a volatile
> function in an update query....
>
> >
> > Not really worse than what the rewriter is doing already --- in fact,
> > I think it's isomorphic to what would happen to the rule qual
> > expressions in your existing patch.
> >
>
> Currently you don't have to rewrite the rule conditions itself every
> time you apply them to the query tree since they are stored in pg_rewrite
> matching all various (reversed) varattno's, resno's and whatever.
> If i understand correctly you need to do that with constraints every time
> you fire an update query on a view for each underlying relation....
>
> >
> > I'm about to propose that we should try to go beta next week (see
> > forthcoming message). If you can strip down your patch to avoid the
> > multi-eval problems in the next couple of days, I'm still willing to
>
> Depends on how many days "couple of days" are.....if you mean the next
> three days
> then definitely not, since i'm afk for the whole upcoming weekend. Bad
> timing, but
> it's not deferrable.... :(
>
> > consider it, but at the moment I'm assuming that it needs to be held
> > for 8.3.
>
> Well, i'll see what i can do next week....it's a little bit disappointing
> that these problems
> raises so late, but that's no one's fault since there are many side effects
> of the rewriting
> system involved....
>
> Many thanks for your comments.
>
> Bernd
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

--On Donnerstag, März 06, 2008 17:03:10 -0500 Bruce Momjian
<bruce(at)momjian(dot)us> wrote:

>
> Where are on updatable views?
>

I really want to have this one ready for 8.4, but i have nothing appliable
at the moment. Considering the amount of rework that needs to be done, i
hope i can provide an updated patch version during next commit fest.

--
Thanks

Bernd

On Thu, Mar 6, 2008 at 6:35 PM, Bernd Helmle <mailings(at)oopsware(dot)de> wrote:
> --On Donnerstag, März 06, 2008 17:03:10 -0500 Bruce Momjian
> <bruce(at)momjian(dot)us> wrote:
>
> >
> > Where are on updatable views?
> >
>
> I really want to have this one ready for 8.4, but i have nothing appliable
> at the moment. Considering the amount of rework that needs to be done, i
> hope i can provide an updated patch version during next commit fest.
>

i will be waiting for it!
maybe you can update the wiki with actual state or with the latest
patch you have even if it doesn't apply... just to see what needs to
be done...

--
regards,
Jaime Casanova