Tables without oids wouldn't be able to be
used inside fk constraints, since some of the checks
in the trigger did a SELECT oid.  Since the oid wasn't
actually used, I changed this to SELECT 1.  My test
case with non-oid tables now works and fk regression
appears to run fine on my machine.
Thu, 8 Nov 2001 09:23:01 -0800 (PST)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Small FK patch to deal with tables without oids
Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> 	Tables without oids wouldn't be able to be
> used inside fk constraints, since some of the checks
> in the trigger did a SELECT oid.  Since the oid wasn't
> actually used, I changed this to SELECT 1.

Can't believe I missed that while looking for OID dependencies :-(
Good catch!

			regards, tom lane

Thu, 08 Nov 2001 23:08:17 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: Small FK patch to deal with tables without oids 

	I wanted to know if there was a decision
to remove the triggered data change violation checks
from trigger.c or to change them to a per statement
check?  I'm building a fix for some foreign key
problems, and want to cover some of those cases
in the regression test if we're going to allow it.



Sun, 11 Nov 2001 10:54:05 -0800 (PST)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Triggered Data Change check
Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> 	I wanted to know if there was a decision
> to remove the triggered data change violation checks
> from trigger.c or to change them to a per statement
> check?

AFAIK no one is happy with the state of that code, but I'm not sure
if we've agreed on exactly how to change it.  Do you have a proposal?

			regards, tom lane

Sun, 11 Nov 2001 14:14:38 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: Triggered Data Change check 
> 
> 	I wanted to know if there was a decision
> to remove the triggered data change violation checks
> from trigger.c or to change them to a per statement
> check?  I'm building a fix for some foreign key
> problems, and want to cover some of those cases
> in the regression test if we're going to allow it.

I would like to do _something_ about that error message, not sure what.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sun, 11 Nov 2001 14:29:59 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Triggered Data Change check

On Sun, 11 Nov 2001, Tom Lane wrote:

> Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> > 	I wanted to know if there was a decision
> > to remove the triggered data change violation checks
> > from trigger.c or to change them to a per statement
> > check?
>
> AFAIK no one is happy with the state of that code, but I'm not sure
> if we've agreed on exactly how to change it.  Do you have a proposal?

Well, I wonder if the check is so weak as to be fairly useless in the
first place really, even if applied to the statement as opposed to the
transaction.  It prevents cases like (not tested, but gives the idea):

create table foo1( a int unique);
create table foo2( a int unique default 2 references foo1(a)
 initially deferred on update set default);
alter table foo1 add foreign key(a) references foo2(a)
 initially deferred on update cascade);
begin;
insert into foo1 values (1);
insert into foo2 values (1);
end;
update foo1 set a=3;
-- I think it would have the following effect:
-- foo1 has "a" set to 3 which sets foo2's "a" to 2 which sets
-- foo1's "a" to 2 as well. And so the row in foo1 is changed
-- twice.

But, since you could do alot of this same work in your own triggers,
and doing so doesn't seem to trip the triggered data change check
(for example an after trigger on a non-fk table that updates the
same table), I wonder if we should either defend against neither
case or both.

As such, I'd say we should at least comment out the check and
error since it would fix alot of cases that people using the
system more normally run into at the expense of a more edge
condition.

One problem is that it opens up the foreign key stuff to a
bunch of cases that haven't been tested before and it may be
a little bit late for opening up that can of worms. I'm confident
that I've fixed related badness in the no action case and on
the base check(since my home copy had the check commented out and the
tests I ran worked in that case), but I haven't touched the referential
actions because they're a little more complicated.


Sun, 11 Nov 2001 12:06:58 -0800 (PST)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Re: Triggered Data Change check 
Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> Well, I wonder if the check is so weak as to be fairly useless in the
> first place really, even if applied to the statement as opposed to the
> transaction.

Looking back at our discussion around 24-Oct, I recall that I was
leaning to the idea that the correct interpretation of the spec's
"triggered data change" rule is that it prohibits scenarios that are
impossible anyway under MVCC, because of the MVCC tuple visibility
rules.  Therefore we don't need any explicit test for triggered data
change.  But I didn't hear anyone else supporting or disproving
that idea.

The code as-is is certainly wrong, since it prohibits multiple changes
within a transaction, not within a statement as the spec says.

Right at the moment I'd favor ripping the code out entirely ... but
it'd be good to hear some support for that approach.  Comments anyone?

			regards, tom lane

Sun, 11 Nov 2001 15:52:47 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: Triggered Data Change check 
Tom Lane wrote:
> 
> Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> > Well, I wonder if the check is so weak as to be fairly useless in the
> > first place really, even if applied to the statement as opposed to the
> > transaction.
> 
> Looking back at our discussion around 24-Oct, I recall that I was
> leaning to the idea that the correct interpretation of the spec's
> "triggered data change" rule is that it prohibits scenarios that are
> impossible anyway under MVCC, because of the MVCC tuple visibility
> rules. 

Strictly speaking MVCC is only for read-only queries.
Even under MVCC, update, delete and select .. for update have
to see the newest tuples. Constraints shouldn't ignore the
update/delete operations in the future from MVCC POV.

regards,
Hiroshi Inoue

Mon, 12 Nov 2001 12:06:49 +0900
Hiroshi Inoue <Inoue@tpf.co.jp>
Re: Triggered Data Change check
Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> Strictly speaking MVCC is only for read-only queries.
> Even under MVCC, update, delete and select .. for update have
> to see the newest tuples.

True.  But my point is that we already have mechanisms to deal with
that set of issues; the trigger code shouldn't concern itself with
the problem.

			regards, tom lane

Sun, 11 Nov 2001 22:11:26 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: Triggered Data Change check 
Tom Lane wrote:
> 
> Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> > Strictly speaking MVCC is only for read-only queries.
> > Even under MVCC, update, delete and select .. for update have
> > to see the newest tuples.
> 
> True.  But my point is that we already have mechanisms to deal with
> that set of issues; the trigger code shouldn't concern itself with
> the problem.

You are saying 
> Therefore we don't need any explicit test for triggered data
> change. 

ISTM your point is on the following.

> Functions can run new commands that get new command ID numbers within
> the current transaction --- but on return from the function, the current
> command number is restored.  I believe rows inserted by such a function
> would look "in the future" to us at the outer command, and would be
> ignored.

My point is why we could ignore the (future) changes. 

regards,
Hiroshi Inoue

Mon, 12 Nov 2001 12:40:59 +0900
Hiroshi Inoue <Inoue@tpf.co.jp>
Re: Triggered Data Change check

On Sun, 11 Nov 2001, Tom Lane wrote:

> Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> > Strictly speaking MVCC is only for read-only queries.
> > Even under MVCC, update, delete and select .. for update have
> > to see the newest tuples.
>
> True.  But my point is that we already have mechanisms to deal with
> that set of issues; the trigger code shouldn't concern itself with
> the problem.

This sequence on my system prints the numbers increasing by 1 which
I would assume means that the updates are going through:

create table foo1(a int);
create function f() returns opaque as 'begin update foo1 set a=a+1; raise
 notice ''%'', NEW.a; return NEW; end;' language 'plpgsql';
create trigger tr after update on foo1 for each row execute
 procedure f();
insert into foo1 values(1);
update foo1 set a=1;

I think that if this were an fk trigger, this would technically be illegal
behavior as soon as that row in foo1 was modified again during the
function execution from the "update foo1 set a=1" statement due to the
following (sql92, 11.8 General Rules -- I don't have the copy of sql99
on this machine to look at, but I'm guessing there's something similar)
         7) If any attempt is made within an SQL-statement to update some
            data item to a value that is distinct from the value to which
            that data item was previously updated within the same SQL-
            statement, then an exception condition is raised: triggered
            data change violation.
Given this is under the referential constraint definition, I'm guessing
it's about ri constraints even though the wording seems to say any
attempt.

Because its easy to get around with general triggers, I'm not sure the
check is meaningful, and it's alot less likely to occur than the normal
update/delete or update/update cases that currently error out in the
system, so I'm also for ripping out the check, although I think we
probably want to think about this for later.


Sun, 11 Nov 2001 19:54:32 -0800 (PST)
Stephan Szabo <sszabo@megazone23.bigpanda.com>
Re: Triggered Data Change check 
Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> My point is why we could ignore the (future) changes. 

We shouldn't.  My feeling is that the various places that consider
HeapTupleSelfUpdated to be an ignorable condition need more thought.
In some cases they should be raising a "data change violation" error,
instead.

It's still not special to triggers, however.  If you read the spec
closely, it's talking about any update not only trigger-caused updates:

         7) If any attempt is made within an SQL-statement to update some
            data item to a value that is distinct from the value to which
            that data item was previously updated within the same SQL-
            statement, then an exception condition is raised: triggered
            data change violation.

It might be that a trigger is the only possible way to make that happen
within SQL92, but we have more ways to make it happen...

			regards, tom lane

Sun, 11 Nov 2001 23:05:48 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: Triggered Data Change check 

Tom, I assume you want this applied?

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

> Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> > 	Tables without oids wouldn't be able to be
> > used inside fk constraints, since some of the checks
> > in the trigger did a SELECT oid.  Since the oid wasn't
> > actually used, I changed this to SELECT 1.
> 
> Can't believe I missed that while looking for OID dependencies :-(
> Good catch!
> 
> 			regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 12 Nov 2001 00:32:26 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Small FK patch to deal with tables without oids
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom, I assume you want this applied?

Please.

			regards, tom lane


> ---------------------------------------------------------------------------

>> Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> Tables without oids wouldn't be able to be
> used inside fk constraints, since some of the checks
> in the trigger did a SELECT oid.  Since the oid wasn't
> actually used, I changed this to SELECT 1.
>> 
>> Can't believe I missed that while looking for OID dependencies :-(
>> Good catch!
>> 
>> regards, tom lane
>> 
>> ---------------------------(end of broadcast)---------------------------
>> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>> 

> -- 
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 12 Nov 2001 01:00:11 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: Small FK patch to deal with tables without oids 

Patch applied.  Thanks.

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


> 
> 	Tables without oids wouldn't be able to be
> used inside fk constraints, since some of the checks
> in the trigger did a SELECT oid.  Since the oid wasn't
> actually used, I changed this to SELECT 1.  My test
> case with non-oid tables now works and fk regression
> appears to run fine on my machine.

Content-Description: 

[ Attachment, skipping... ]

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 12 Nov 2001 01:09:03 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Small FK patch to deal with tables without oids
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom, I assume you want this applied?
> 
> Please.

Done.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 12 Nov 2001 01:09:11 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Small FK patch to deal with tables without oids