Lists: | pgsql-hackers |
---|
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | minimal update |
Date: | 2007-11-02 15:49:38 |
Message-ID: | 472B4712.3060704@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
For some time I have been working on removing some inefficiencies from a
large DW-type app. This app does a large daily batch update, and this is
what is the major bottleneck. One of the things I have been doing is to
remove unnecessary updates (which are particualrly expensive in our
index-rich setting). Several times now I have wished that there was a
switch on the UPDATE command that said "do minimal instead of maximal
updating". i.e., don't update records with identical replacements. At
the moment I have to write things like:
update tname set foo = bar ... where foo is null or foo <> bar ...
This becomes more than tedious when the update might be setting thirty
or forty fields, and I have to write such tests for each of them. It
would be so much nicer to be able to write something like:
update tname minimally set foo = bar ...
Is this an insane idea, or would it be possible, practical and useful?
cheers
andrew
From: | David Fetter <david(at)fetter(dot)org> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-02 16:17:04 |
Message-ID: | 20071102161704.GC3913@fetter.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Fri, Nov 02, 2007 at 11:49:38AM -0400, Andrew Dunstan wrote:
> For some time I have been working on removing some inefficiencies
> from a large DW-type app. This app does a large daily batch update,
> and this is what is the major bottleneck. One of the things I have
> been doing is to remove unnecessary updates (which are particualrly
> expensive in our index-rich setting). Several times now I have
> wished that there was a switch on the UPDATE command that said "do
> minimal instead of maximal updating". i.e., don't update records
> with identical replacements. At the moment I have to write things
> like:
>
> update tname set foo = bar ... where foo is null or foo <> bar
> ...
One way I've done this is make RULEs which basically drop non-updating
"UPDATEs" on the floor.
CREATE RULE foo_drop_empty_updates AS
ON UPDATE TO foo
WHERE ROW(OLD.*)::foo IS NOT DISTINCT FROM ROW(NEW.*)::foo
DO INSTEAD NOTHING;
It's pretty easy to automate rule creation, but since Postgres doesn't
have DDL triggers, it's also a bit of a foot gun.
By the way, the above has what I think of as an infelicity in 8.2.5,
namely that you need non-obvious contortions to get it to work. I'm
thinking OLD IS NOT DISTINCT FROM NEW should Just Work(TM).
> This becomes more than tedious when the update might be setting thirty
> or forty fields, and I have to write such tests for each of them. It
> would be so much nicer to be able to write something like:
>
> update tname minimally set foo = bar ...
>
> Is this an insane idea, or would it be possible, practical and useful?
I don't know about the sanity, but I've done it a couple of places :)
Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | David Fetter <david(at)fetter(dot)org> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-02 16:25:48 |
Message-ID: | 7164.1194020748@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
David Fetter <david(at)fetter(dot)org> writes:
> On Fri, Nov 02, 2007 at 11:49:38AM -0400, Andrew Dunstan wrote:
>> At the moment I have to write things like:
>>
>> update tname set foo = bar ... where foo is null or foo <> bar
> One way I've done this is make RULEs which basically drop non-updating
> "UPDATEs" on the floor.
A BEFORE UPDATE trigger would be better, and probably hardly more
expensive than a wired-in facility (especially if you were willing to
write it in C).
regards, tom lane
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-02 17:14:02 |
Message-ID: | 472B5ADA.7050203@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> David Fetter <david(at)fetter(dot)org> writes:
>
>> On Fri, Nov 02, 2007 at 11:49:38AM -0400, Andrew Dunstan wrote:
>>
>>> At the moment I have to write things like:
>>>
>>> update tname set foo = bar ... where foo is null or foo <> bar
>>>
>
>
>> One way I've done this is make RULEs which basically drop non-updating
>> "UPDATEs" on the floor.
>>
>
> A BEFORE UPDATE trigger would be better, and probably hardly more
> expensive than a wired-in facility (especially if you were willing to
> write it in C).
>
>
>
Yes. I also prefer the trigger idea to a rule because triggers are easy
to enable and disable. It's still a lot of work for what must be a
common want, though. Could it be done generically?
cheers
andrew
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-02 17:29:02 |
Message-ID: | 11736.1194024542@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> A BEFORE UPDATE trigger would be better, and probably hardly more
>> expensive than a wired-in facility (especially if you were willing to
>> write it in C).
> Yes. I also prefer the trigger idea to a rule because triggers are easy
> to enable and disable. It's still a lot of work for what must be a
> common want, though. Could it be done generically?
Well, you could write the trigger in C and it'd work for any table.
I think it could be as simple as a memcmp of the tuples' data areas,
since we now require padding bytes to be 0 ...
regards, tom lane
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-02 17:44:11 |
Message-ID: | 472B61EB.50604@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> Tom Lane wrote:
>>
>>> A BEFORE UPDATE trigger would be better, and probably hardly more
>>> expensive than a wired-in facility (especially if you were willing to
>>> write it in C).
>>>
>
>
>> Yes. I also prefer the trigger idea to a rule because triggers are easy
>> to enable and disable. It's still a lot of work for what must be a
>> common want, though. Could it be done generically?
>>
>
> Well, you could write the trigger in C and it'd work for any table.
> I think it could be as simple as a memcmp of the tuples' data areas,
> since we now require padding bytes to be 0 ...
>
>
>
Ah. Good. Thanks, that's the piece I was missing.
cheers
andrew
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-05 15:03:03 |
Message-ID: | 472F30A7.6070505@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> Tom Lane wrote:
>>
>>> A BEFORE UPDATE trigger would be better, and probably hardly more
>>> expensive than a wired-in facility (especially if you were willing to
>>> write it in C).
>>>
>
>
>> Yes. I also prefer the trigger idea to a rule because triggers are easy
>> to enable and disable. It's still a lot of work for what must be a
>> common want, though. Could it be done generically?
>>
>
> Well, you could write the trigger in C and it'd work for any table.
> I think it could be as simple as a memcmp of the tuples' data areas,
> since we now require padding bytes to be 0 ...
>
>
>
Something like this fragment?
newtuple = trigdata->tg_newtuple;
oldtuple = trigdata->tg_trigtuple;
rettuple = newtuple;
if (newtuple->t_len == oldtuple->t_len &&
newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
memcmp(GETSTRUCT(newtuple),GETSTRUCT(oldtuple),
newtuple->t_len - newtuple->t_data->t_hoff) == 0)
rettuple = NULL;
return PointerGetDatum(rettuple);
Also, when did we first require padding bytes to be 0?
cheers
andrew
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-05 15:29:55 |
Message-ID: | 20070.1194276595@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> Well, you could write the trigger in C and it'd work for any table.
>> I think it could be as simple as a memcmp of the tuples' data areas,
>> since we now require padding bytes to be 0 ...
> Something like this fragment?
> newtuple = trigdata->tg_newtuple;
> oldtuple = trigdata->tg_trigtuple;
> rettuple = newtuple;
> if (newtuple->t_len == oldtuple->t_len &&
> newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
> memcmp(GETSTRUCT(newtuple),GETSTRUCT(oldtuple),
> newtuple->t_len - newtuple->t_data->t_hoff) == 0)
> rettuple = NULL;
> return PointerGetDatum(rettuple);
Close, but I think you also need to take care to compare natts and
the null bitmaps (if any). Might be worth comparing OIDs too, though
AFAIR there is no mechanism for substituting a different OID during
UPDATE. Probably the easiest coding is to memcmp all the way from
offsetof(t_bits) to t_len, after comparing natts and the HASNULL and
HASOID flags.
> Also, when did we first require padding bytes to be 0?
The 8.3 varvarlena patch is what requires it, but in practice
heap_formtuple has always started with a palloc0, so I think it would
work a long ways back.
regards, tom lane
From: | Michael Glaesemann <grzm(at)seespotcode(dot)net> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-08 11:57:33 |
Message-ID: | 381FC610-9CD2-4885-BE3A-66F4E5DA3744@seespotcode.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Nov 2, 2007, at 13:44 , Andrew Dunstan wrote:
> Ah. Good. Thanks, that's the piece I was missing.
What would be the disadvantages of always doing this, i.e., just
making this part of the normal update path in the backend? I'd think
it should save on unnecessarily dead tuples as well.
Michael Glaesemann
grzm seespotcode net
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Michael Glaesemann <grzm(at)seespotcode(dot)net> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-08 15:07:41 |
Message-ID: | 24321.1194534461@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Michael Glaesemann <grzm(at)seespotcode(dot)net> writes:
> What would be the disadvantages of always doing this, i.e., just
> making this part of the normal update path in the backend?
(1) cycles wasted to no purpose in the vast majority of cases.
(2) visibly inconsistent behavior for apps that pay attention
to ctid/xmin/etc.
(3) visibly inconsistent behavior for apps that have AFTER triggers.
There's enough other overhead in issuing an update (network,
parsing/planning/etc) that a sanely coded application should try
to avoid issuing no-op updates anyway. The proposed trigger is
just a band-aid IMHO.
I think having it as an optional trigger is a reasonable compromise.
regards, tom lane
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-08 15:46:58 |
Message-ID: | 47332F72.8040809@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Michael Glaesemann <grzm(at)seespotcode(dot)net> writes:
>
>> What would be the disadvantages of always doing this, i.e., just
>> making this part of the normal update path in the backend?
>>
>
> (1) cycles wasted to no purpose in the vast majority of cases.
>
> (2) visibly inconsistent behavior for apps that pay attention
> to ctid/xmin/etc.
>
> (3) visibly inconsistent behavior for apps that have AFTER triggers.
>
> There's enough other overhead in issuing an update (network,
> parsing/planning/etc) that a sanely coded application should try
> to avoid issuing no-op updates anyway. The proposed trigger is
> just a band-aid IMHO.
>
> I think having it as an optional trigger is a reasonable compromise.
>
>
>
Right. I never proposed making this the default behaviour, for all these
good reasons.
The point about making the app try to avoid no-op updates is that this
can impose some quite considerable code complexity on the app,
especially where the number of updated fields is large. It's fragile and
error-prone. A simple switch that can turn a trigger on or off will be
nicer. Syntax support for that might be even nicer, but there appears to
be some resistance to that, so I can easily settle for the trigger.
cheers
andrew
From: | Michael Glaesemann <grzm(at)seespotcode(dot)net> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-10 13:41:49 |
Message-ID: | 5761C0C5-80DB-4602-A28C-91003127C87F@seespotcode.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Nov 8, 2007, at 10:46 , Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
>> Michael Glaesemann <grzm(at)seespotcode(dot)net> writes:
>>
>>> What would be the disadvantages of always doing this, i.e., just
>>> making this part of the normal update path in the backend?
>>>
>>
>> (1) cycles wasted to no purpose in the vast majority of cases.
>>
>> (2) visibly inconsistent behavior for apps that pay attention
>> to ctid/xmin/etc.
>>
>> (3) visibly inconsistent behavior for apps that have AFTER triggers.
>>
>> There's enough other overhead in issuing an update (network,
>> parsing/planning/etc) that a sanely coded application should try
>> to avoid issuing no-op updates anyway. The proposed trigger is
>> just a band-aid IMHO.
>>
>> I think having it as an optional trigger is a reasonable compromise.
>>
>>
>>
>
> Right. I never proposed making this the default behaviour, for all
> these good reasons.
>
> The point about making the app try to avoid no-op updates is that
> this can impose some quite considerable code complexity on the app,
> especially where the number of updated fields is large. It's
> fragile and error-prone. A simple switch that can turn a trigger on
> or off will be nicer. Syntax support for that might be even nicer,
> but there appears to be some resistance to that, so I can easily
> settle for the trigger.
This confirms what I thought. Thanks.
Michael Glaesemann
grzm seespotcode net
From: | Decibel! <decibel(at)decibel(dot)org> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-12 15:54:16 |
Message-ID: | 2287072E-57C6-4FCE-A37D-306B4DB8B20D@decibel.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Nov 2, 2007, at 10:49 AM, Andrew Dunstan wrote:
> update tname set foo = bar ... where foo is null or foo <> bar ...
FYI, you should be able to do WHERE foo IS DISTINCT FROM bar instead.
--
Decibel!, aka Jim C. Nasby, Database Architect decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | "Decibel!" <decibel(at)decibel(dot)org> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-11-12 16:02:46 |
Message-ID: | 47387926.9050305@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Decibel! wrote:
> On Nov 2, 2007, at 10:49 AM, Andrew Dunstan wrote:
>> update tname set foo = bar ... where foo is null or foo <> bar ...
>
> FYI, you should be able to do WHERE foo IS DISTINCT FROM bar instead.
True, that's a bit nicer. It's still more than somewhat ugly and fragile
if there a lot of foos and the bars are complex expressions.
cheers
andrew
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-12-28 22:56:56 |
Message-ID: | 47757F38.8060509@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> Tom Lane wrote:
>>
>>> Well, you could write the trigger in C and it'd work for any table.
>>> I think it could be as simple as a memcmp of the tuples' data areas,
>>> since we now require padding bytes to be 0 ...
>>>
>
>
>> Something like this fragment?
>>
>
>
>> newtuple = trigdata->tg_newtuple;
>> oldtuple = trigdata->tg_trigtuple;
>> rettuple = newtuple;
>>
>
>
>> if (newtuple->t_len == oldtuple->t_len &&
>> newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
>> memcmp(GETSTRUCT(newtuple),GETSTRUCT(oldtuple),
>> newtuple->t_len - newtuple->t_data->t_hoff) == 0)
>> rettuple = NULL;
>>
>
>
>> return PointerGetDatum(rettuple);
>>
>
> Close, but I think you also need to take care to compare natts and
> the null bitmaps (if any). Might be worth comparing OIDs too, though
> AFAIR there is no mechanism for substituting a different OID during
> UPDATE. Probably the easiest coding is to memcmp all the way from
> offsetof(t_bits) to t_len, after comparing natts and the HASNULL and
> HASOID flags.
>
How does this look?
if (newtuple->t_len == oldtuple->t_len &&
newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
HeapTupleHeaderGetNatts(newtuple) == HeapTupleHeaderGetNatts(oldtuple) &&
(newtuple->t_data->t_infomask & (HEAP_HASOID|HEAP_HASNULL)) == (oldtuple->t_data->t_infomask & (HEAP_HASOID|HEAP_HASNULL)) &&
memcmp(newtuple->t_data + offsetof(HeapTupleHeaderData, t_bits),
oldtuple->t_data + offsetof(HeapTupleHeaderData, t_bits)
newtuple->t_len - offsetof(HeapTupleHeaderData, t_bits)) == 0)
rettuple = NULL;
return PointerGetDatum(rettuple);
cheers
andrew
>
>> Also, when did we first require padding bytes to be 0?
>>
>
> The 8.3 varvarlena patch is what requires it, but in practice
> heap_formtuple has always started with a palloc0, so I think it would
> work a long ways back.
>
> regards, tom lane
>
>
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-12-28 23:24:54 |
Message-ID: | 18370.1198884294@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> How does this look?
> if (newtuple->t_len == oldtuple->t_len &&
> newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
> HeapTupleHeaderGetNatts(newtuple) == HeapTupleHeaderGetNatts(oldtuple) &&
> (newtuple->t_data->t_infomask & (HEAP_HASOID|HEAP_HASNULL)) == (oldtuple->t_data->t_infomask & (HEAP_HASOID|HEAP_HASNULL)) &&
> memcmp(newtuple->t_data + offsetof(HeapTupleHeaderData, t_bits),
> oldtuple->t_data + offsetof(HeapTupleHeaderData, t_bits)
> newtuple->t_len - offsetof(HeapTupleHeaderData, t_bits)) == 0)
> rettuple = NULL;
Looks sane. It might be even saner if you compare all of the
non-visibility-related infomask bits, viz
(newtuple->t_data->t_infomask & ~HEAP_XACT_MASK) ==
(oldtuple->t_data->t_infomask & ~HEAP_XACT_MASK)
rather than just HASOID and HASNULL.
regards, tom lane
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-12-29 01:26:06 |
Message-ID: | 4775A22E.6090306@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> How does this look?
>>
>
>
>> if (newtuple->t_len == oldtuple->t_len &&
>> newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
>> HeapTupleHeaderGetNatts(newtuple) == HeapTupleHeaderGetNatts(oldtuple) &&
>> (newtuple->t_data->t_infomask & (HEAP_HASOID|HEAP_HASNULL)) == (oldtuple->t_data->t_infomask & (HEAP_HASOID|HEAP_HASNULL)) &&
>> memcmp(newtuple->t_data + offsetof(HeapTupleHeaderData, t_bits),
>> oldtuple->t_data + offsetof(HeapTupleHeaderData, t_bits)
>> newtuple->t_len - offsetof(HeapTupleHeaderData, t_bits)) == 0)
>>
>
>
>> rettuple = NULL;
>>
>
> Looks sane. It might be even saner if you compare all of the
> non-visibility-related infomask bits, viz
>
> (newtuple->t_data->t_infomask & ~HEAP_XACT_MASK) ==
> (oldtuple->t_data->t_infomask & ~HEAP_XACT_MASK)
>
> rather than just HASOID and HASNULL.
>
>
>
Sadly, the memcmp is failing on my test ("update foo set bar = bar") on
8.2. Looks like I'm in for weekend with my fave debugger :-(
cheers
andrew
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2007-12-29 05:40:13 |
Message-ID: | 4775DDBD.4050005@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>
>>> How does this look?
>>>
>>
>>
>>> if (newtuple->t_len == oldtuple->t_len &&
>>> newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
>>> HeapTupleHeaderGetNatts(newtuple) ==
>>> HeapTupleHeaderGetNatts(oldtuple) &&
>>> (newtuple->t_data->t_infomask & (HEAP_HASOID|HEAP_HASNULL))
>>> == (oldtuple->t_data->t_infomask & (HEAP_HASOID|HEAP_HASNULL)) &&
>>> memcmp(newtuple->t_data + offsetof(HeapTupleHeaderData,
>>> t_bits),
>>> oldtuple->t_data + offsetof(HeapTupleHeaderData, t_bits)
>>> newtuple->t_len - offsetof(HeapTupleHeaderData,
>>> t_bits)) == 0)
>>>
>>
>>
>>> rettuple = NULL;
>>>
>>
>> Looks sane. It might be even saner if you compare all of the
>> non-visibility-related infomask bits, viz
>>
>> (newtuple->t_data->t_infomask & ~HEAP_XACT_MASK) ==
>> (oldtuple->t_data->t_infomask & ~HEAP_XACT_MASK)
>>
>> rather than just HASOID and HASNULL.
>>
>>
>>
>
> Sadly, the memcmp is failing on my test ("update foo set bar = bar")
> on 8.2. Looks like I'm in for weekend with my fave debugger :-(
>
>
Turns out we needed those pointers used in the arguments to memcmp cast
to char * so the pointer arithmetic would work right.
I'll be suggesting we add a utility function like this for 8.4.
cheers
andrew
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-03-07 16:10:41 |
Message-ID: | 200803071610.m27GAfV00596@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
I assume don't want a TODO for this? (Suppress UPDATE no changed
columns)
---------------------------------------------------------------------------
Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
> > Michael Glaesemann <grzm(at)seespotcode(dot)net> writes:
> >
> >> What would be the disadvantages of always doing this, i.e., just
> >> making this part of the normal update path in the backend?
> >>
> >
> > (1) cycles wasted to no purpose in the vast majority of cases.
> >
> > (2) visibly inconsistent behavior for apps that pay attention
> > to ctid/xmin/etc.
> >
> > (3) visibly inconsistent behavior for apps that have AFTER triggers.
> >
> > There's enough other overhead in issuing an update (network,
> > parsing/planning/etc) that a sanely coded application should try
> > to avoid issuing no-op updates anyway. The proposed trigger is
> > just a band-aid IMHO.
> >
> > I think having it as an optional trigger is a reasonable compromise.
> >
> >
> >
>
> Right. I never proposed making this the default behaviour, for all these
> good reasons.
>
> The point about making the app try to avoid no-op updates is that this
> can impose some quite considerable code complexity on the app,
> especially where the number of updated fields is large. It's fragile and
> error-prone. A simple switch that can turn a trigger on or off will be
> nicer. Syntax support for that might be even nicer, but there appears to
> be some resistance to that, so I can easily settle for the trigger.
>
> cheers
>
> andrew
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org
--
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. +
From: | "Gurjeet Singh" <singh(dot)gurjeet(at)gmail(dot)com> |
---|---|
To: | "Bruce Momjian" <bruce(at)momjian(dot)us> |
Cc: | "Andrew Dunstan" <andrew(at)dunslane(dot)net>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Michael Glaesemann" <grzm(at)seespotcode(dot)net>, "David Fetter" <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-03-18 13:31:26 |
Message-ID: | 65937bea0803180631o78d44cd7n2928834bcdb101fd@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> I assume don't want a TODO for this? (Suppress UPDATE no changed
> columns)
>
I am starting to implement this. Do we want to have this trigger function in
the server, or in an external module?
Best regards,
>
>
> ---------------------------------------------------------------------------
>
> Andrew Dunstan wrote:
> >
> >
> > Tom Lane wrote:
> > > Michael Glaesemann <grzm(at)seespotcode(dot)net> writes:
> > >
> > >> What would be the disadvantages of always doing this, i.e., just
> > >> making this part of the normal update path in the backend?
> > >>
> > >
> > > (1) cycles wasted to no purpose in the vast majority of cases.
> > >
> > > (2) visibly inconsistent behavior for apps that pay attention
> > > to ctid/xmin/etc.
> > >
> > > (3) visibly inconsistent behavior for apps that have AFTER triggers.
> > >
> > > There's enough other overhead in issuing an update (network,
> > > parsing/planning/etc) that a sanely coded application should try
> > > to avoid issuing no-op updates anyway. The proposed trigger is
> > > just a band-aid IMHO.
> > >
> > > I think having it as an optional trigger is a reasonable compromise.
> > >
> > >
> > >
> >
> > Right. I never proposed making this the default behaviour, for all these
> > good reasons.
> >
> > The point about making the app try to avoid no-op updates is that this
> > can impose some quite considerable code complexity on the app,
> > especially where the number of updated fields is large. It's fragile and
> > error-prone. A simple switch that can turn a trigger on or off will be
> > nicer. Syntax support for that might be even nicer, but there appears to
> > be some resistance to that, so I can easily settle for the trigger.
> >
> > cheers
> >
> > andrew
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 4: Have you searched our list archives?
> >
> > http://archives.postgresql.org
>
> --
> 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. +
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
--
gurjeet[(dot)singh](at)EnterpriseDB(dot)com
singh(dot)gurjeet(at){ gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
17° 29' 34.37"N, 78° 30' 59.76"E - Hyderabad *
18° 32' 57.25"N, 73° 56' 25.42"E - Pune
37° 47' 19.72"N, 122° 24' 1.69" W - San Francisco
Mail sent from my BlackLaptop device
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> |
Cc: | Bruce Momjian <bruce(at)momjian(dot)us>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-03-18 14:16:13 |
Message-ID: | 47DFCEAD.4020908@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Gurjeet Singh wrote:
> On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian <bruce(at)momjian(dot)us
> <mailto:bruce(at)momjian(dot)us>> wrote:
>
>
> I assume don't want a TODO for this? (Suppress UPDATE no changed
> columns)
>
>
> I am starting to implement this. Do we want to have this trigger
> function in the server, or in an external module?
>
>
I have the trigger part of this done, in fact. What remains to be done
is to add it to the catalog and document it. The intention is to make it
a builtin as it will be generally useful. If you want to work on the
remaining parts then I will happily ship you the C code for the trigger.
cheers
andrew
From: | "Gurjeet Singh" <singh(dot)gurjeet(at)gmail(dot)com> |
---|---|
To: | "Andrew Dunstan" <andrew(at)dunslane(dot)net> |
Cc: | "Bruce Momjian" <bruce(at)momjian(dot)us>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Michael Glaesemann" <grzm(at)seespotcode(dot)net>, "David Fetter" <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-03-18 14:46:38 |
Message-ID: | 65937bea0803180746s5ec87000gc755adfefece1441@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
>
>
>
> Gurjeet Singh wrote:
> > On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian <bruce(at)momjian(dot)us
> > <mailto:bruce(at)momjian(dot)us>> wrote:
> >
> >
> > I assume don't want a TODO for this? (Suppress UPDATE no changed
> > columns)
> >
> >
> > I am starting to implement this. Do we want to have this trigger
> > function in the server, or in an external module?
> >
> >
>
> I have the trigger part of this done, in fact. What remains to be done
> is to add it to the catalog and document it. The intention is to make it
> a builtin as it will be generally useful. If you want to work on the
> remaining parts then I will happily ship you the C code for the trigger.
>
>
In fact, I just finished writing the C code and including it in the catalog
(Just tested that it's visible in the catalog). I will test it to see if it
does actually do what we want it to.
I have incorporated all the suggestions above. Would love to see your code
in the meantime.
Here's the C code:
Datum
trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
{
TriggerData *trigData;
HeapTuple oldTuple;
HeapTuple newTuple;
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, "trig_ignore_duplicate_updates: not called by trigger
manager.");
if( !TRIGGER_FIRED_BY_UPDATE(trigData->tg_event)
&& !TRIGGER_FIRED_BEFORE(trigData->tg_event)
&& !TRIGGER_FIRED_FOR_ROW(trigData->tg_event) )
{
elog(ERROR, "trig_ignore_duplicate_updates: Can only be executed for
UPDATE, BEFORE and FOR EACH ROW.");
}
trigData = (TriggerData *) fcinfo->context;
oldTuple = trigData->tg_trigtuple;
newTuple = trigData->tg_newtuple;
if (newTuple->t_len == oldTuple->t_len
&& newTuple->t_data->t_hoff == oldTuple->t_data->t_hoff
&& HeapTupleHeaderGetNatts(newTuple->t_data) ==
HeapTupleHeaderGetNatts(oldTuple->t_data)
&& (newTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
== (oldTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
&& memcmp( (char*)(newTuple->t_data) + offsetof(HeapTupleHeaderData,
t_bits),
(char*)(oldTuple->t_data) + offsetof(HeapTupleHeaderData,
t_bits),
newTuple->t_len - offsetof(HeapTupleHeaderData, t_bits)
) == 0 )
{
/* return without crating a new tuple */
return PointerGetDatum( NULL );
}
return PointerGetDatum( trigData->tg_newtuple );
}
--
gurjeet[(dot)singh](at)EnterpriseDB(dot)com
singh(dot)gurjeet(at){ gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
17° 29' 34.37"N, 78° 30' 59.76"E - Hyderabad *
18° 32' 57.25"N, 73° 56' 25.42"E - Pune
37° 47' 19.72"N, 122° 24' 1.69" W - San Francisco
Mail sent from my BlackLaptop device
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-05-08 00:34:20 |
Message-ID: | 200805080034.m480YKw11292@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Is there a version of this patch ready for application?
---------------------------------------------------------------------------
Gurjeet Singh wrote:
> On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
> >
> >
> >
> >
> >
> > Gurjeet Singh wrote:
> > > On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian <bruce(at)momjian(dot)us
> > > <mailto:bruce(at)momjian(dot)us>> wrote:
> > >
> > >
> > > I assume don't want a TODO for this? (Suppress UPDATE no changed
> > > columns)
> > >
> > >
> > > I am starting to implement this. Do we want to have this trigger
> > > function in the server, or in an external module?
> > >
> > >
> >
> > I have the trigger part of this done, in fact. What remains to be done
> > is to add it to the catalog and document it. The intention is to make it
> > a builtin as it will be generally useful. If you want to work on the
> > remaining parts then I will happily ship you the C code for the trigger.
> >
> >
> In fact, I just finished writing the C code and including it in the catalog
> (Just tested that it's visible in the catalog). I will test it to see if it
> does actually do what we want it to.
>
> I have incorporated all the suggestions above. Would love to see your code
> in the meantime.
>
> Here's the C code:
>
> Datum
> trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
> {
> TriggerData *trigData;
> HeapTuple oldTuple;
> HeapTuple newTuple;
>
> if (!CALLED_AS_TRIGGER(fcinfo))
> elog(ERROR, "trig_ignore_duplicate_updates: not called by trigger
> manager.");
>
> if( !TRIGGER_FIRED_BY_UPDATE(trigData->tg_event)
> && !TRIGGER_FIRED_BEFORE(trigData->tg_event)
> && !TRIGGER_FIRED_FOR_ROW(trigData->tg_event) )
> {
> elog(ERROR, "trig_ignore_duplicate_updates: Can only be executed for
> UPDATE, BEFORE and FOR EACH ROW.");
> }
>
> trigData = (TriggerData *) fcinfo->context;
> oldTuple = trigData->tg_trigtuple;
> newTuple = trigData->tg_newtuple;
>
> if (newTuple->t_len == oldTuple->t_len
> && newTuple->t_data->t_hoff == oldTuple->t_data->t_hoff
> && HeapTupleHeaderGetNatts(newTuple->t_data) ==
> HeapTupleHeaderGetNatts(oldTuple->t_data)
> && (newTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
> == (oldTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
> && memcmp( (char*)(newTuple->t_data) + offsetof(HeapTupleHeaderData,
> t_bits),
> (char*)(oldTuple->t_data) + offsetof(HeapTupleHeaderData,
> t_bits),
> newTuple->t_len - offsetof(HeapTupleHeaderData, t_bits)
> ) == 0 )
> {
> /* return without crating a new tuple */
> return PointerGetDatum( NULL );
> }
>
> return PointerGetDatum( trigData->tg_newtuple );
> }
>
>
>
> --
> gurjeet[(dot)singh](at)EnterpriseDB(dot)com
> singh(dot)gurjeet(at){ gmail | hotmail | indiatimes | yahoo }.com
>
> EnterpriseDB http://www.enterprisedb.com
>
> 17? 29' 34.37"N, 78? 30' 59.76"E - Hyderabad *
> 18? 32' 57.25"N, 73? 56' 25.42"E - Pune
> 37? 47' 19.72"N, 122? 24' 1.69" W - San Francisco
>
> http://gurjeet.frihost.net
>
> Mail sent from my BlackLaptop device
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-05-08 00:38:02 |
Message-ID: | 48224B6A.9010400@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Not that I know of. I never saw Gurjeet's completed code.
cheers
andrew
Bruce Momjian wrote:
> Is there a version of this patch ready for application?
>
> ---------------------------------------------------------------------------
>
> Gurjeet Singh wrote:
>
>> On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>
>>
>>>
>>>
>>>
>>> Gurjeet Singh wrote:
>>>
>>>> On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian <bruce(at)momjian(dot)us
>>>> <mailto:bruce(at)momjian(dot)us>> wrote:
>>>>
>>>>
>>>> I assume don't want a TODO for this? (Suppress UPDATE no changed
>>>> columns)
>>>>
>>>>
>>>> I am starting to implement this. Do we want to have this trigger
>>>> function in the server, or in an external module?
>>>>
>>>>
>>>>
>>> I have the trigger part of this done, in fact. What remains to be done
>>> is to add it to the catalog and document it. The intention is to make it
>>> a builtin as it will be generally useful. If you want to work on the
>>> remaining parts then I will happily ship you the C code for the trigger.
>>>
>>>
>>>
>> In fact, I just finished writing the C code and including it in the catalog
>> (Just tested that it's visible in the catalog). I will test it to see if it
>> does actually do what we want it to.
>>
>> I have incorporated all the suggestions above. Would love to see your code
>> in the meantime.
>>
>> Here's the C code:
>>
>> Datum
>> trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
>> {
>> TriggerData *trigData;
>> HeapTuple oldTuple;
>> HeapTuple newTuple;
>>
>> if (!CALLED_AS_TRIGGER(fcinfo))
>> elog(ERROR, "trig_ignore_duplicate_updates: not called by trigger
>> manager.");
>>
>> if( !TRIGGER_FIRED_BY_UPDATE(trigData->tg_event)
>> && !TRIGGER_FIRED_BEFORE(trigData->tg_event)
>> && !TRIGGER_FIRED_FOR_ROW(trigData->tg_event) )
>> {
>> elog(ERROR, "trig_ignore_duplicate_updates: Can only be executed for
>> UPDATE, BEFORE and FOR EACH ROW.");
>> }
>>
>> trigData = (TriggerData *) fcinfo->context;
>> oldTuple = trigData->tg_trigtuple;
>> newTuple = trigData->tg_newtuple;
>>
>> if (newTuple->t_len == oldTuple->t_len
>> && newTuple->t_data->t_hoff == oldTuple->t_data->t_hoff
>> && HeapTupleHeaderGetNatts(newTuple->t_data) ==
>> HeapTupleHeaderGetNatts(oldTuple->t_data)
>> && (newTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
>> == (oldTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
>> && memcmp( (char*)(newTuple->t_data) + offsetof(HeapTupleHeaderData,
>> t_bits),
>> (char*)(oldTuple->t_data) + offsetof(HeapTupleHeaderData,
>> t_bits),
>> newTuple->t_len - offsetof(HeapTupleHeaderData, t_bits)
>> ) == 0 )
>> {
>> /* return without crating a new tuple */
>> return PointerGetDatum( NULL );
>> }
>>
>> return PointerGetDatum( trigData->tg_newtuple );
>> }
>>
>>
>>
>> --
>> gurjeet[(dot)singh](at)EnterpriseDB(dot)com
>> singh(dot)gurjeet(at){ gmail | hotmail | indiatimes | yahoo }.com
>>
>> EnterpriseDB http://www.enterprisedb.com
>>
>> 17? 29' 34.37"N, 78? 30' 59.76"E - Hyderabad *
>> 18? 32' 57.25"N, 73? 56' 25.42"E - Pune
>> 37? 47' 19.72"N, 122? 24' 1.69" W - San Francisco
>>
>> http://gurjeet.frihost.net
>>
>> Mail sent from my BlackLaptop device
>>
>
>
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-05-08 01:16:56 |
Message-ID: | 200805080116.m481GuH15769@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan wrote:
>
> Not that I know of. I never saw Gurjeet's completed code.
This is Gurjeet's code, but it is not complete.
http://archives.postgresql.org/pgsql-hackers/2008-03/msg00668.php
---------------------------------------------------------------------------
>
> cheers
>
> andrew
>
> Bruce Momjian wrote:
> > Is there a version of this patch ready for application?
> >
> > ---------------------------------------------------------------------------
> >
> > Gurjeet Singh wrote:
> >
> >> On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> >>
> >>
> >>>
> >>>
> >>>
> >>> Gurjeet Singh wrote:
> >>>
> >>>> On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian <bruce(at)momjian(dot)us
> >>>> <mailto:bruce(at)momjian(dot)us>> wrote:
> >>>>
> >>>>
> >>>> I assume don't want a TODO for this? (Suppress UPDATE no changed
> >>>> columns)
> >>>>
> >>>>
> >>>> I am starting to implement this. Do we want to have this trigger
> >>>> function in the server, or in an external module?
> >>>>
> >>>>
> >>>>
> >>> I have the trigger part of this done, in fact. What remains to be done
> >>> is to add it to the catalog and document it. The intention is to make it
> >>> a builtin as it will be generally useful. If you want to work on the
> >>> remaining parts then I will happily ship you the C code for the trigger.
> >>>
> >>>
> >>>
> >> In fact, I just finished writing the C code and including it in the catalog
> >> (Just tested that it's visible in the catalog). I will test it to see if it
> >> does actually do what we want it to.
> >>
> >> I have incorporated all the suggestions above. Would love to see your code
> >> in the meantime.
> >>
> >> Here's the C code:
> >>
> >> Datum
> >> trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
> >> {
> >> TriggerData *trigData;
> >> HeapTuple oldTuple;
> >> HeapTuple newTuple;
> >>
> >> if (!CALLED_AS_TRIGGER(fcinfo))
> >> elog(ERROR, "trig_ignore_duplicate_updates: not called by trigger
> >> manager.");
> >>
> >> if( !TRIGGER_FIRED_BY_UPDATE(trigData->tg_event)
> >> && !TRIGGER_FIRED_BEFORE(trigData->tg_event)
> >> && !TRIGGER_FIRED_FOR_ROW(trigData->tg_event) )
> >> {
> >> elog(ERROR, "trig_ignore_duplicate_updates: Can only be executed for
> >> UPDATE, BEFORE and FOR EACH ROW.");
> >> }
> >>
> >> trigData = (TriggerData *) fcinfo->context;
> >> oldTuple = trigData->tg_trigtuple;
> >> newTuple = trigData->tg_newtuple;
> >>
> >> if (newTuple->t_len == oldTuple->t_len
> >> && newTuple->t_data->t_hoff == oldTuple->t_data->t_hoff
> >> && HeapTupleHeaderGetNatts(newTuple->t_data) ==
> >> HeapTupleHeaderGetNatts(oldTuple->t_data)
> >> && (newTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
> >> == (oldTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
> >> && memcmp( (char*)(newTuple->t_data) + offsetof(HeapTupleHeaderData,
> >> t_bits),
> >> (char*)(oldTuple->t_data) + offsetof(HeapTupleHeaderData,
> >> t_bits),
> >> newTuple->t_len - offsetof(HeapTupleHeaderData, t_bits)
> >> ) == 0 )
> >> {
> >> /* return without crating a new tuple */
> >> return PointerGetDatum( NULL );
> >> }
> >>
> >> return PointerGetDatum( trigData->tg_newtuple );
> >> }
> >>
> >>
> >>
> >> --
> >> gurjeet[(dot)singh](at)EnterpriseDB(dot)com
> >> singh(dot)gurjeet(at){ gmail | hotmail | indiatimes | yahoo }.com
> >>
> >> EnterpriseDB http://www.enterprisedb.com
> >>
> >> 17? 29' 34.37"N, 78? 30' 59.76"E - Hyderabad *
> >> 18? 32' 57.25"N, 73? 56' 25.42"E - Pune
> >> 37? 47' 19.72"N, 122? 24' 1.69" W - San Francisco
> >>
> >> http://gurjeet.frihost.net
> >>
> >> Mail sent from my BlackLaptop device
> >>
> >
> >
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-05-08 01:25:01 |
Message-ID: | 4822566D.6020804@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Right. In fact, I already had that part in fact - see
http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html
What I was waiting for was the part where it gets put in the catalog,
documented, etc.
cheers
andrew
Bruce Momjian wrote:
> Andrew Dunstan wrote:
>
>> Not that I know of. I never saw Gurjeet's completed code.
>>
>
> This is Gurjeet's code, but it is not complete.
>
> http://archives.postgresql.org/pgsql-hackers/2008-03/msg00668.php
>
> ---------------------------------------------------------------------------
>
>
>> cheers
>>
>> andrew
>>
>> Bruce Momjian wrote:
>>
>>> Is there a version of this patch ready for application?
>>>
>>> ---------------------------------------------------------------------------
>>>
>>> Gurjeet Singh wrote:
>>>
>>>
>>>> On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>>>
>>>>
>>>>
>>>>>
>>>>> Gurjeet Singh wrote:
>>>>>
>>>>>
>>>>>> On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian <bruce(at)momjian(dot)us
>>>>>> <mailto:bruce(at)momjian(dot)us>> wrote:
>>>>>>
>>>>>>
>>>>>> I assume don't want a TODO for this? (Suppress UPDATE no changed
>>>>>> columns)
>>>>>>
>>>>>>
>>>>>> I am starting to implement this. Do we want to have this trigger
>>>>>> function in the server, or in an external module?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> I have the trigger part of this done, in fact. What remains to be done
>>>>> is to add it to the catalog and document it. The intention is to make it
>>>>> a builtin as it will be generally useful. If you want to work on the
>>>>> remaining parts then I will happily ship you the C code for the trigger.
>>>>>
>>>>>
>>>>>
>>>>>
>>>> In fact, I just finished writing the C code and including it in the catalog
>>>> (Just tested that it's visible in the catalog). I will test it to see if it
>>>> does actually do what we want it to.
>>>>
>>>> I have incorporated all the suggestions above. Would love to see your code
>>>> in the meantime.
>>>>
>>>> Here's the C code:
>>>>
>>>> Datum
>>>> trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
>>>> {
>>>> TriggerData *trigData;
>>>> HeapTuple oldTuple;
>>>> HeapTuple newTuple;
>>>>
>>>> if (!CALLED_AS_TRIGGER(fcinfo))
>>>> elog(ERROR, "trig_ignore_duplicate_updates: not called by trigger
>>>> manager.");
>>>>
>>>> if( !TRIGGER_FIRED_BY_UPDATE(trigData->tg_event)
>>>> && !TRIGGER_FIRED_BEFORE(trigData->tg_event)
>>>> && !TRIGGER_FIRED_FOR_ROW(trigData->tg_event) )
>>>> {
>>>> elog(ERROR, "trig_ignore_duplicate_updates: Can only be executed for
>>>> UPDATE, BEFORE and FOR EACH ROW.");
>>>> }
>>>>
>>>> trigData = (TriggerData *) fcinfo->context;
>>>> oldTuple = trigData->tg_trigtuple;
>>>> newTuple = trigData->tg_newtuple;
>>>>
>>>> if (newTuple->t_len == oldTuple->t_len
>>>> && newTuple->t_data->t_hoff == oldTuple->t_data->t_hoff
>>>> && HeapTupleHeaderGetNatts(newTuple->t_data) ==
>>>> HeapTupleHeaderGetNatts(oldTuple->t_data)
>>>> && (newTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
>>>> == (oldTuple->t_data->t_infomask & ~HEAP_XACT_MASK)
>>>> && memcmp( (char*)(newTuple->t_data) + offsetof(HeapTupleHeaderData,
>>>> t_bits),
>>>> (char*)(oldTuple->t_data) + offsetof(HeapTupleHeaderData,
>>>> t_bits),
>>>> newTuple->t_len - offsetof(HeapTupleHeaderData, t_bits)
>>>> ) == 0 )
>>>> {
>>>> /* return without crating a new tuple */
>>>> return PointerGetDatum( NULL );
>>>> }
>>>>
>>>> return PointerGetDatum( trigData->tg_newtuple );
>>>> }
>>>>
>>>>
>>>>
>>>> --
>>>> gurjeet[(dot)singh](at)EnterpriseDB(dot)com
>>>> singh(dot)gurjeet(at){ gmail | hotmail | indiatimes | yahoo }.com
>>>>
>>>> EnterpriseDB http://www.enterprisedb.com
>>>>
>>>> 17? 29' 34.37"N, 78? 30' 59.76"E - Hyderabad *
>>>> 18? 32' 57.25"N, 73? 56' 25.42"E - Pune
>>>> 37? 47' 19.72"N, 122? 24' 1.69" W - San Francisco
>>>>
>>>> http://gurjeet.frihost.net
>>>>
>>>> Mail sent from my BlackLaptop device
>>>>
>>>>
>>>
>>>
>
>
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-05-08 01:29:22 |
Message-ID: | 200805080129.m481TMn17305@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan wrote:
>
> Right. In fact, I already had that part in fact - see
> http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html
>
> What I was waiting for was the part where it gets put in the catalog,
> documented, etc.
I can probably do that part. Send over what you have and I will work on
it. Thanks.
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-05-08 01:36:06 |
Message-ID: | 48225906.2050201@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Bruce Momjian wrote:
> Andrew Dunstan wrote:
>
>> Right. In fact, I already had that part in fact - see
>> http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html
>>
>> What I was waiting for was the part where it gets put in the catalog,
>> documented, etc.
>>
>
> I can probably do that part. Send over what you have and I will work on
> it. Thanks.
>
>
It's very similar to what Gurjeet posted (but designed to work with
earlier postgres versions)
cheers
andrew
---
|#include "postgres.h"
#include "commands/trigger.h"
#include "access/htup.h"
#ifdef PG_MODULE_MAGIC
PG_MODULE_MAGIC;
#endif
/* for pre 8.3 */
#ifndef HeapTupleHeaderGetNatts
#define HeapTupleHeaderGetNatts(th) ( (th)->t_natts )
#endif
extern Datum min_update_trigger(PG_FUNCTION_ARGS);
PG_FUNCTION_INFO_V1(min_update_trigger);
Datum
min_update_trigger(PG_FUNCTION_ARGS)
{
TriggerData *trigdata = (TriggerData *) fcinfo->context;
HeapTuple newtuple, oldtuple, rettuple;
/* make sure it's called as a trigger at all */
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, "min_update_trigger: not called by trigger manager");
/* and that it's called on update */
if (! TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
elog(ERROR, "min_update_trigger: not called on update");
/* and that it's called before update */
if (! TRIGGER_FIRED_BEFORE(trigdata->tg_event))
elog(ERROR, "min_update_trigger: not called before update");
/* and that it's called for each row */
if (! TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
elog(ERROR, "min_update_trigger: not called for each row");
/* get tuple dat, set default return */
rettuple = newtuple = trigdata->tg_newtuple;
oldtuple = trigdata->tg_trigtuple;
if (newtuple->t_len == oldtuple->t_len &&
newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
HeapTupleHeaderGetNatts(newtuple->t_data) == HeapTupleHeaderGetNatts(oldtuple->t_data) &&
(newtuple->t_data->t_infomask & ~HEAP_XACT_MASK) ==
(oldtuple->t_data->t_infomask & ~HEAP_XACT_MASK) &&
memcmp(((char *)newtuple->t_data) + offsetof(HeapTupleHeaderData, t_bits),
((char *)oldtuple->t_data) + offsetof(HeapTupleHeaderData, t_bits),
newtuple->t_len - offsetof(HeapTupleHeaderData, t_bits)) == 0)
rettuple = NULL;
return PointerGetDatum(rettuple);
}|
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-15 00:29:28 |
Message-ID: | 48F53968.1010909@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Bruce,
did you ever look at completing this?
cheers
andrew
Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
>> Andrew Dunstan wrote:
>>
>>> Right. In fact, I already had that part in fact - see
>>> http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html
>>>
>>>
>>> What I was waiting for was the part where it gets put in the
>>> catalog, documented, etc.
>>>
>>
>> I can probably do that part. Send over what you have and I will work on
>> it. Thanks.
>>
>>
>
> It's very similar to what Gurjeet posted (but designed to work with
> earlier postgres versions)
>
> cheers
>
> andrew
>
> ---
>
> |#include "postgres.h"
> #include "commands/trigger.h"
> #include "access/htup.h"
>
> #ifdef PG_MODULE_MAGIC
> PG_MODULE_MAGIC;
> #endif
>
> /* for pre 8.3 */
> #ifndef HeapTupleHeaderGetNatts
> #define HeapTupleHeaderGetNatts(th) ( (th)->t_natts )
> #endif
>
> extern Datum min_update_trigger(PG_FUNCTION_ARGS);
>
> PG_FUNCTION_INFO_V1(min_update_trigger);
>
> Datum
> min_update_trigger(PG_FUNCTION_ARGS)
> {
> TriggerData *trigdata = (TriggerData *) fcinfo->context;
> HeapTuple newtuple, oldtuple, rettuple;
>
> /* make sure it's called as a trigger at all */
> if (!CALLED_AS_TRIGGER(fcinfo))
> elog(ERROR, "min_update_trigger: not called by trigger manager");
>
> /* and that it's called on update */
> if (! TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
> elog(ERROR, "min_update_trigger: not called on update");
>
> /* and that it's called before update */
> if (! TRIGGER_FIRED_BEFORE(trigdata->tg_event))
> elog(ERROR, "min_update_trigger: not called before update");
>
> /* and that it's called for each row */
> if (! TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
> elog(ERROR, "min_update_trigger: not called for each row");
>
> /* get tuple dat, set default return */
> rettuple = newtuple = trigdata->tg_newtuple;
> oldtuple = trigdata->tg_trigtuple;
>
> if (newtuple->t_len == oldtuple->t_len &&
> newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
> HeapTupleHeaderGetNatts(newtuple->t_data) ==
> HeapTupleHeaderGetNatts(oldtuple->t_data) &&
> (newtuple->t_data->t_infomask & ~HEAP_XACT_MASK) ==
> (oldtuple->t_data->t_infomask & ~HEAP_XACT_MASK) &&
> memcmp(((char *)newtuple->t_data) +
> offsetof(HeapTupleHeaderData, t_bits),
> ((char *)oldtuple->t_data) +
> offsetof(HeapTupleHeaderData, t_bits),
> newtuple->t_len -
> offsetof(HeapTupleHeaderData, t_bits)) == 0)
> rettuple = NULL;
>
> return PointerGetDatum(rettuple);
> }|
>
>
>
>
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-16 02:42:11 |
Message-ID: | 200810160242.m9G2gBF06628@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan wrote:
>
> Bruce,
>
> did you ever look at completing this?
No, it is still in my email box unaddressed. Feel free to work on it; I
doubt I can do it for 8.4.
---------------------------------------------------------------------------
>
> cheers
>
> andrew
>
> Andrew Dunstan wrote:
> >
> >
> > Bruce Momjian wrote:
> >> Andrew Dunstan wrote:
> >>
> >>> Right. In fact, I already had that part in fact - see
> >>> http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html
> >>>
> >>>
> >>> What I was waiting for was the part where it gets put in the
> >>> catalog, documented, etc.
> >>>
> >>
> >> I can probably do that part. Send over what you have and I will work on
> >> it. Thanks.
> >>
> >>
> >
> > It's very similar to what Gurjeet posted (but designed to work with
> > earlier postgres versions)
> >
> > cheers
> >
> > andrew
> >
> > ---
> >
> > |#include "postgres.h"
> > #include "commands/trigger.h"
> > #include "access/htup.h"
> >
> > #ifdef PG_MODULE_MAGIC
> > PG_MODULE_MAGIC;
> > #endif
> >
> > /* for pre 8.3 */
> > #ifndef HeapTupleHeaderGetNatts
> > #define HeapTupleHeaderGetNatts(th) ( (th)->t_natts )
> > #endif
> >
> > extern Datum min_update_trigger(PG_FUNCTION_ARGS);
> >
> > PG_FUNCTION_INFO_V1(min_update_trigger);
> >
> > Datum
> > min_update_trigger(PG_FUNCTION_ARGS)
> > {
> > TriggerData *trigdata = (TriggerData *) fcinfo->context;
> > HeapTuple newtuple, oldtuple, rettuple;
> >
> > /* make sure it's called as a trigger at all */
> > if (!CALLED_AS_TRIGGER(fcinfo))
> > elog(ERROR, "min_update_trigger: not called by trigger manager");
> >
> > /* and that it's called on update */
> > if (! TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
> > elog(ERROR, "min_update_trigger: not called on update");
> >
> > /* and that it's called before update */
> > if (! TRIGGER_FIRED_BEFORE(trigdata->tg_event))
> > elog(ERROR, "min_update_trigger: not called before update");
> >
> > /* and that it's called for each row */
> > if (! TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
> > elog(ERROR, "min_update_trigger: not called for each row");
> >
> > /* get tuple dat, set default return */
> > rettuple = newtuple = trigdata->tg_newtuple;
> > oldtuple = trigdata->tg_trigtuple;
> >
> > if (newtuple->t_len == oldtuple->t_len &&
> > newtuple->t_data->t_hoff == oldtuple->t_data->t_hoff &&
> > HeapTupleHeaderGetNatts(newtuple->t_data) ==
> > HeapTupleHeaderGetNatts(oldtuple->t_data) &&
> > (newtuple->t_data->t_infomask & ~HEAP_XACT_MASK) ==
> > (oldtuple->t_data->t_infomask & ~HEAP_XACT_MASK) &&
> > memcmp(((char *)newtuple->t_data) +
> > offsetof(HeapTupleHeaderData, t_bits),
> > ((char *)oldtuple->t_data) +
> > offsetof(HeapTupleHeaderData, t_bits),
> > newtuple->t_len -
> > offsetof(HeapTupleHeaderData, t_bits)) == 0)
> > rettuple = NULL;
> >
> > return PointerGetDatum(rettuple);
> > }|
> >
> >
> >
> >
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-16 17:37:57 |
Message-ID: | 48F77BF5.2010101@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Bruce Momjian wrote:
> Andrew Dunstan wrote:
>
>> Bruce,
>>
>> did you ever look at completing this?
>>
>
> No, it is still in my email box unaddressed. Feel free to work on it; I
> doubt I can do it for 8.4.
>
>
>
OK. Where would be a good place to put the code? Maybe a new file
src/backend/utils/adt/trigger_utils.c ?
cheers
andrew
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-16 17:44:46 |
Message-ID: | 17813.1224179086@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> OK. Where would be a good place to put the code? Maybe a new file
> src/backend/utils/adt/trigger_utils.c ?
I thought the plan was to make it a contrib module.
regards, tom lane
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-16 18:03:26 |
Message-ID: | 48F781EE.3040407@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> OK. Where would be a good place to put the code? Maybe a new file
>> src/backend/utils/adt/trigger_utils.c ?
>>
>
> I thought the plan was to make it a contrib module.
>
>
>
Well, previous discussion did mention catalog entries, which would
suggest otherwise, but I can do it as a contrib module if that's the
consensus.
cheers
andrew
From: | Magnus Hagander <magnus(at)hagander(dot)net> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-20 08:28:35 |
Message-ID: | 48FC4133.7020708@hagander.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>
>>> OK. Where would be a good place to put the code? Maybe a new file
>>> src/backend/utils/adt/trigger_utils.c ?
>>>
>>
>> I thought the plan was to make it a contrib module.
>>
>>
>>
>
> Well, previous discussion did mention catalog entries, which would
> suggest otherwise, but I can do it as a contrib module if that's the
> consensus.
What would be the actual reason to put it in contrib and not core? Are
there any "dangers" by having it there? Or is it "just a hack" and not a
"real solution"?
//Magnus
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Magnus Hagander <magnus(at)hagander(dot)net> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-20 14:51:54 |
Message-ID: | 48FC9B0A.4000502@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Magnus Hagander wrote:
> Andrew Dunstan wrote:
>
>> Tom Lane wrote:
>>
>>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>>
>>>
>>>> OK. Where would be a good place to put the code? Maybe a new file
>>>> src/backend/utils/adt/trigger_utils.c ?
>>>>
>>>>
>>> I thought the plan was to make it a contrib module.
>>>
>>>
>>>
>>>
>> Well, previous discussion did mention catalog entries, which would
>> suggest otherwise, but I can do it as a contrib module if that's the
>> consensus.
>>
>
> What would be the actual reason to put it in contrib and not core? Are
> there any "dangers" by having it there? Or is it "just a hack" and not a
> "real solution"?
>
>
>
No, it's not just a hack. It's very close to what we'd probably do if we
built the facility right into the language, although it does involve the
overhead of calling the trigger. However, it performs reasonably well -
not surprising since the guts of it is just a memcmp() call.
cheers
andrew
From: | Magnus Hagander <magnus(at)hagander(dot)net> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-21 13:34:04 |
Message-ID: | EBBA1874-C593-4602-AB4E-6BE57C4B2E67@hagander.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 20 okt 2008, at 16.51, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
> Magnus Hagander wrote:
>> Andrew Dunstan wrote:
>>
>>> Tom Lane wrote:
>>>
>>>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>>>
>>>>> OK. Where would be a good place to put the code? Maybe a new file
>>>>> src/backend/utils/adt/trigger_utils.c ?
>>>>>
>>>> I thought the plan was to make it a contrib module.
>>>>
>>>>
>>> Well, previous discussion did mention catalog entries, which would
>>> suggest otherwise, but I can do it as a contrib module if that's the
>>> consensus.
>>>
>>
>> What would be the actual reason to put it in contrib and not core?
>> Are
>> there any "dangers" by having it there? Or is it "just a hack" and
>> not a
>> "real solution"?
>>
>>
>>
>
> No, it's not just a hack. It's very close to what we'd probably do
> if we built the facility right into the language, although it does
> involve the overhead of calling the trigger. However, it performs
> reasonably well - not surprising since the guts of it is just a
> memcmp() call.
>
In that case, why not put the trigger in core so people can use it
easily?
/magnus
From: | David Fetter <david(at)fetter(dot)org> |
---|---|
To: | Magnus Hagander <magnus(at)hagander(dot)net> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-21 13:36:30 |
Message-ID: | 20081021133630.GE1906@fetter.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Tue, Oct 21, 2008 at 03:34:04PM +0200, Magnus Hagander wrote:
> On 20 okt 2008, at 16.51, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>> Magnus Hagander wrote:
>>> Andrew Dunstan wrote:
>>>> Tom Lane wrote:
>>>>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>>>>> OK. Where would be a good place to put the code? Maybe a new
>>>>>> file src/backend/utils/adt/trigger_utils.c ?
>>>>>>
>>>>> I thought the plan was to make it a contrib module.
>>>>>
>>>> Well, previous discussion did mention catalog entries, which
>>>> would suggest otherwise, but I can do it as a contrib module if
>>>> that's the consensus.
>>>
>>> What would be the actual reason to put it in contrib and not core?
>>> Are there any "dangers" by having it there? Or is it "just a hack"
>>> and not a "real solution"?
>>
>> No, it's not just a hack. It's very close to what we'd probably do
>> if we built the facility right into the language, although it does
>> involve the overhead of calling the trigger. However, it performs
>> reasonably well - not surprising since the guts of it is just a
>> memcmp() call.
>>
> In that case, why not put the trigger in core so people can use it
> easily?
+1 :)
Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
From: | "Merlin Moncure" <mmoncure(at)gmail(dot)com> |
---|---|
To: | "Magnus Hagander" <magnus(at)hagander(dot)net> |
Cc: | "Andrew Dunstan" <andrew(at)dunslane(dot)net>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Bruce Momjian" <bruce(at)momjian(dot)us>, "Gurjeet Singh" <singh(dot)gurjeet(at)gmail(dot)com>, "Michael Glaesemann" <grzm(at)seespotcode(dot)net>, "David Fetter" <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-21 13:39:09 |
Message-ID: | b42b73150810210639k255559dek93675d852ec3da09@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Tue, Oct 21, 2008 at 9:34 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On 20 okt 2008, at 16.51, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>
>> No, it's not just a hack. It's very close to what we'd probably do if we
>> built the facility right into the language, although it does involve the
>> overhead of calling the trigger. However, it performs reasonably well - not
>> surprising since the guts of it is just a memcmp() call.
>>
> In that case, why not put the trigger in core so people can use it easily?
+1
This is hard to get right and a common source of errors.
merlin
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Magnus Hagander <magnus(at)hagander(dot)net> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-21 13:40:56 |
Message-ID: | 19167.1224596456@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Magnus Hagander <magnus(at)hagander(dot)net> writes:
> In that case, why not put the trigger in core so people can use it
> easily?
One advantage of making it a contrib module is that discussing how/when
to use it would fit more easily into the structure of the
documentation. There is no place in our docs that a "standard trigger"
would fit without seeming like a wart; but a contrib module can document
itself pretty much however it wants.
regards, tom lane
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-21 14:11:41 |
Message-ID: | 48FDE31D.5040200@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>
>> In that case, why not put the trigger in core so people can use it
>> easily?
>>
>
> One advantage of making it a contrib module is that discussing how/when
> to use it would fit more easily into the structure of the
> documentation. There is no place in our docs that a "standard trigger"
> would fit without seeming like a wart; but a contrib module can document
> itself pretty much however it wants.
>
I was thinking a new section on 'trigger functions' of the functions and
operators chapter, linked from the 'create trigger' page. That doesn't
seem like too much of a wart.
cheers
andrew
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-22 18:43:24 |
Message-ID: | 48FF744C.5070708@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan wrote:
>
>
> Tom Lane wrote:
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>
>>> In that case, why not put the trigger in core so people can use it
>>> easily?
>>>
>>
>> One advantage of making it a contrib module is that discussing how/when
>> to use it would fit more easily into the structure of the
>> documentation. There is no place in our docs that a "standard trigger"
>> would fit without seeming like a wart; but a contrib module can document
>> itself pretty much however it wants.
>>
>
> I was thinking a new section on 'trigger functions' of the functions
> and operators chapter, linked from the 'create trigger' page. That
> doesn't seem like too much of a wart.
>
>
There seems to be a preponderance of opinion for doing this as a
builtin. Here is a patch that does it that way, along with docs and
regression test.
cheers
andrew
Attachment | Content-Type | Size |
---|---|---|
min-update.patch | text/x-patch | 8.9 KB |
From: | "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> |
---|---|
To: | "Andrew Dunstan" <andrew(at)dunslane(dot)net>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | "David Fetter" <david(at)fetter(dot)org>, "Gurjeet Singh" <singh(dot)gurjeet(at)gmail(dot)com>, "Magnus Hagander" <magnus(at)hagander(dot)net>, "Bruce Momjian" <bruce(at)momjian(dot)us>, "PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>, "Michael Glaesemann" <grzm(at)seespotcode(dot)net> |
Subject: | Re: minimal update |
Date: | 2008-10-22 18:56:53 |
Message-ID: | 48FF3124.EE98.0025.0@wicourts.gov |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
>>> Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> Here is a patch that does it that way, along with docs
s/mare/more/
-Kevin
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Michael Glaesemann <grzm(at)seespotcode(dot)net> |
Subject: | Re: minimal update |
Date: | 2008-10-22 19:15:54 |
Message-ID: | 48FF7BEA.1060301@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Kevin Grittner wrote:
>>>> Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>>>
>
>
>> Here is a patch that does it that way, along with docs
>>
>
> s/mare/more/
>
>
Thanks. fixed in my tree..
cheers
andrew
From: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-22 19:21:06 |
Message-ID: | 1224703266.27145.441.camel@ebony.2ndQuadrant |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Wed, 2008-10-22 at 14:43 -0400, Andrew Dunstan wrote:
>
> There seems to be a preponderance of opinion for doing this as a
> builtin. Here is a patch that does it that way, along with docs and
> regression test.
In your example you use an underscore as the first character. The way
you have done this it will probably exclude any other before row
triggers from firing, which may have altered the value of one or more
columns. The more probable choice for me would be to have a trigger that
came after all other before triggers, and so should have a different
name. It's just an example, so your choice is fine, but I think you
should bring out that point more clearly for the average developer.
Can we call the function "minimal_update_trigger", rather than min_...
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-22 19:24:35 |
Message-ID: | 11392.1224703475@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> Can we call the function "minimal_update_trigger", rather than min_...
"Minimal" really fails to convey the point here IMHO. How about
something like "suppress_no_op_updates_trigger"?
regards, tom lane
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-22 19:27:02 |
Message-ID: | 48FF7E86.4050404@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Simon Riggs wrote:
> On Wed, 2008-10-22 at 14:43 -0400, Andrew Dunstan wrote:
>
>
>
>> There seems to be a preponderance of opinion for doing this as a
>> builtin. Here is a patch that does it that way, along with docs and
>> regression test.
>>
>
> In your example you use an underscore as the first character. The way
> you have done this it will probably exclude any other before row
> triggers from firing, which may have altered the value of one or more
> columns. The more probable choice for me would be to have a trigger that
> came after all other before triggers, and so should have a different
> name. It's just an example, so your choice is fine, but I think you
> should bring out that point more clearly for the average developer.
>
Fair point. I'll add that to the docs.
> Can we call the function "minimal_update_trigger", rather than min_...
>
>
If that's the general consensus, sure. I have no strong opinion.
cheers
andrew
From: | "Robert Haas" <robertmhaas(at)gmail(dot)com> |
---|---|
To: | "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | "Simon Riggs" <simon(at)2ndquadrant(dot)com>, "Andrew Dunstan" <andrew(at)dunslane(dot)net>, "Magnus Hagander" <magnus(at)hagander(dot)net>, "Bruce Momjian" <bruce(at)momjian(dot)us>, "Gurjeet Singh" <singh(dot)gurjeet(at)gmail(dot)com>, "Michael Glaesemann" <grzm(at)seespotcode(dot)net>, "David Fetter" <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-22 21:24:30 |
Message-ID: | 603c8f070810221424m71c234a6ld694e2675ee591b2@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> Can we call the function "minimal_update_trigger", rather than min_...
>
> "Minimal" really fails to convey the point here IMHO. How about
> something like "suppress_no_op_updates_trigger"?
+1. That's a much better name.
...Robert
From: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-22 21:56:37 |
Message-ID: | 1224712597.27145.528.camel@ebony.2ndQuadrant |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Wed, 2008-10-22 at 17:24 -0400, Robert Haas wrote:
> On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> >> Can we call the function "minimal_update_trigger", rather than min_...
> >
> > "Minimal" really fails to convey the point here IMHO. How about
> > something like "suppress_no_op_updates_trigger"?
>
> +1. That's a much better name.
>
I think it means something to us, but "no op" is a very technical phrase
that probably doesn't travel very well. Not everybody Majored in Comp
Sci and speaks Amglish as their native language.
Certainly this intention is much better than "minimal", but a more
widely acceptable phrase is probably better. I will avoid trying to come
up with something myself though.
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-22 22:05:26 |
Message-ID: | 27462.1224713126@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> "Minimal" really fails to convey the point here IMHO. How about
>>> something like "suppress_no_op_updates_trigger"?
> I think it means something to us, but "no op" is a very technical phrase
> that probably doesn't travel very well.
Agreed --- I was hoping someone could improve on that part. The only
other words I could come up with were "empty" and "useless", neither of
which seem quite le mot juste ...
regards, tom lane
From: | Kenneth Marshall <ktm(at)rice(dot)edu> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-22 22:10:55 |
Message-ID: | 20081022221055.GS27578@it.is.rice.edu |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Wed, Oct 22, 2008 at 06:05:26PM -0400, Tom Lane wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> >> On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >>> "Minimal" really fails to convey the point here IMHO. How about
> >>> something like "suppress_no_op_updates_trigger"?
>
> > I think it means something to us, but "no op" is a very technical phrase
> > that probably doesn't travel very well.
>
> Agreed --- I was hoping someone could improve on that part. The only
> other words I could come up with were "empty" and "useless", neither of
> which seem quite le mot juste ...
>
> regards, tom lane
>
redundant?
Ken
From: | "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> |
---|---|
To: | "Simon Riggs" <simon(at)2ndQuadrant(dot)com>, "Robert Haas" <robertmhaas(at)gmail(dot)com> |
Cc: | "Andrew Dunstan" <andrew(at)dunslane(dot)net>, "David Fetter" <david(at)fetter(dot)org>, "Gurjeet Singh" <singh(dot)gurjeet(at)gmail(dot)com>, "Magnus Hagander" <magnus(at)hagander(dot)net>, "Bruce Momjian" <bruce(at)momjian(dot)us>, "PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>, "Michael Glaesemann" <grzm(at)seespotcode(dot)net>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: minimal update |
Date: | 2008-10-22 22:11:50 |
Message-ID: | 48FF5ED6.EE98.0025.0@wicourts.gov |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
>>> Simon Riggs <simon(at)2ndQuadrant(dot)com> wrote:
>> On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
wrote:
>> > How about
>> > something like "suppress_no_op_updates_trigger"?
>
> I think it means something to us, but "no op" is a very technical
phrase
> that probably doesn't travel very well. Not everybody Majored in
Comp
> Sci and speaks Amglish as their native language.
>
> Certainly this intention is much better than "minimal", but a more
> widely acceptable phrase is probably better. I will avoid trying to
come
> up with something myself though.
How about one of these?:
suppress_same_value_updates_trigger
suppress_no_change_updates_trigger
suppress_no_effect_updates_trigger
They all seem a bit awkward, but the best that came to mind.
-Kevin
From: | "Robert Haas" <robertmhaas(at)gmail(dot)com> |
---|---|
To: | "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> |
Cc: | "Simon Riggs" <simon(at)2ndquadrant(dot)com>, "Andrew Dunstan" <andrew(at)dunslane(dot)net>, "David Fetter" <david(at)fetter(dot)org>, "Gurjeet Singh" <singh(dot)gurjeet(at)gmail(dot)com>, "Magnus Hagander" <magnus(at)hagander(dot)net>, "Bruce Momjian" <bruce(at)momjian(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, "Michael Glaesemann" <grzm(at)seespotcode(dot)net>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: minimal update |
Date: | 2008-10-22 22:57:19 |
Message-ID: | 603c8f070810221557g17a0ef94p627b4b39355d186f@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
> How about one of these?:
> suppress_same_value_updates_trigger
> suppress_no_change_updates_trigger
> suppress_no_effect_updates_trigger
I like the first one. A trigger firing would be an "effect", and
possibly a "change", but "same value" seems very clear.
...Robert
From: | Decibel! <decibel(at)decibel(dot)org> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-25 05:38:10 |
Message-ID: | 150717AA-3427-4FC4-99CD-547B6505AA69@decibel.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Oct 22, 2008, at 1:43 PM, Andrew Dunstan wrote:
> + if (!CALLED_AS_TRIGGER(fcinfo))
> + elog(ERROR, "min_update_trigger: not called by trigger
> manager");
The error I get in 8.2 when calling a trigger function directly is:
ERROR: trigger functions may only be called as triggers
To stay consistent, I think the remaining errors should s/: not/ may
only be/, ie:
min_update_trigger may only be called on update
> + /* and that it's called on update */
> + if (! TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
> + elog(ERROR, "min_update_trigger: not called on update");
> +
> + /* and that it's called before update */
> + if (! TRIGGER_FIRED_BEFORE(trigdata->tg_event))
> + elog(ERROR, "min_update_trigger: not called before update");
> +
> + /* and that it's called for each row */
> + if (! TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
> + elog(ERROR, "min_update_trigger: not called for each row");
--
Decibel!, aka Jim C. Nasby, Database Architect decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Kenneth Marshall <ktm(at)rice(dot)edu> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-29 19:31:09 |
Message-ID: | 4908B9FD.3040905@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Kenneth Marshall wrote:
> On Wed, Oct 22, 2008 at 06:05:26PM -0400, Tom Lane wrote:
>
>> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>>
>>>> On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>
>>>>> "Minimal" really fails to convey the point here IMHO. How about
>>>>> something like "suppress_no_op_updates_trigger"?
>>>>>
>>> I think it means something to us, but "no op" is a very technical phrase
>>> that probably doesn't travel very well.
>>>
>> Agreed --- I was hoping someone could improve on that part. The only
>> other words I could come up with were "empty" and "useless", neither of
>> which seem quite le mot juste ...
>>
>> regards, tom lane
>>
>>
> redundant?
>
>
>
I think I like this best of all the suggestions -
suppress_redundant_updates_trigger() is what I have now.
If there's no further discussion, I'll go ahead and commit this in a day
or two.
cheers
andrew
Attachment | Content-Type | Size |
---|---|---|
min-update-2.patch | text/x-patch | 15.4 KB |
From: | Magnus Hagander <magnus(at)hagander(dot)net> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Kenneth Marshall <ktm(at)rice(dot)edu>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-29 19:34:35 |
Message-ID: | 4908BACB.7040205@hagander.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan wrote:
>
>
> Kenneth Marshall wrote:
>> On Wed, Oct 22, 2008 at 06:05:26PM -0400, Tom Lane wrote:
>>
>>> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>>>
>>>>> On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>>
>>>>>> "Minimal" really fails to convey the point here IMHO. How about
>>>>>> something like "suppress_no_op_updates_trigger"?
>>>>>>
>>>> I think it means something to us, but "no op" is a very technical
>>>> phrase
>>>> that probably doesn't travel very well.
>>>>
>>> Agreed --- I was hoping someone could improve on that part. The only
>>> other words I could come up with were "empty" and "useless", neither of
>>> which seem quite le mot juste ...
>>>
>>> regards, tom lane
>>>
>>>
>> redundant?
>>
>>
>>
>
> I think I like this best of all the suggestions -
> suppress_redundant_updates_trigger() is what I have now.
>
> If there's no further discussion, I'll go ahead and commit this in a day
> or two.
Nitpicking, but you have:
+ <para>
+ Currently <productname>PostgreSQL</> provides one built in trigger
+ function, <function>suppress_redundant_updates_trigger</>,
Should we perhaps mention the fulltext triggers (with the appropriate
links) here? If it's intended to be an authoritative list of the
"userspace" triggers we ship, I think that may be a good idea.
//Magnus
From: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Kenneth Marshall <ktm(at)rice(dot)edu>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-29 19:35:21 |
Message-ID: | 20081029193521.GJ4331@alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan escribió:
> + /* make sure it's called as a trigger */
> + if (!CALLED_AS_TRIGGER(fcinfo))
> + elog(ERROR, "suppress_redundant_updates_trigger: must be called as trigger");
Shouldn't these all be ereport()?
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
Cc: | Kenneth Marshall <ktm(at)rice(dot)edu>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-29 19:48:09 |
Message-ID: | 4908BDF9.8020205@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Alvaro Herrera wrote:
> Andrew Dunstan escribió:
>
>
>> + /* make sure it's called as a trigger */
>> + if (!CALLED_AS_TRIGGER(fcinfo))
>> + elog(ERROR, "suppress_redundant_updates_trigger: must be called as trigger");
>>
>
> Shouldn't these all be ereport()?
>
>
Good point.
I'll fix them.
Maybe we should fix our C sample trigger, from which this was taken.
cheers
andrew
From: | David Fetter <david(at)fetter(dot)org> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Kenneth Marshall <ktm(at)rice(dot)edu>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-29 20:08:29 |
Message-ID: | 20081029200829.GA18097@fetter.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Wed, Oct 29, 2008 at 03:48:09PM -0400, Andrew Dunstan wrote:
>
>>> + /* make sure it's called as a trigger */
>>> + if (!CALLED_AS_TRIGGER(fcinfo))
>>> + elog(ERROR, "suppress_redundant_updates_trigger: must be called as trigger");
>>
>> Shouldn't these all be ereport()?
>
> Good point.
>
> I'll fix them.
>
> Maybe we should fix our C sample trigger, from which this was taken.
Yes :)
Does the attached have the right error code?
Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment | Content-Type | Size |
---|---|---|
trigger_ereport.diff | text/plain | 607 bytes |
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | David Fetter <david(at)fetter(dot)org> |
Cc: | Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Kenneth Marshall <ktm(at)rice(dot)edu>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-29 20:20:15 |
Message-ID: | 4908C57F.3060203@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
David Fetter wrote:
>>
>> Maybe we should fix our C sample trigger, from which this was taken.
>>
>
> Yes :)
>
> Does the attached have the right error code?
>
> - elog(ERROR, "trigf: not called by trigger manager");
> + ereport(ERROR,
> + (error(TRIGGERED_DATA_CHANGE_VIOLATION),
> + errmsg("trigf: not called by trigger manager")));
>
>
Not sure that's appropriate, but I can't see anything else that is very
appropriate either.
cheers
andrew
From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | David Fetter <david(at)fetter(dot)org> |
Cc: | Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Kenneth Marshall <ktm(at)rice(dot)edu>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-29 20:36:06 |
Message-ID: | 4908C936.4070105@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan wrote:
>
>
> David Fetter wrote:
>>>
>>> Maybe we should fix our C sample trigger, from which this was taken.
>>>
>>
>> Yes :)
>>
>> Does the attached have the right error code?
>>
>> - elog(ERROR, "trigf: not called by trigger manager");
>> + ereport(ERROR,
>> + (error(TRIGGERED_DATA_CHANGE_VIOLATION),
>> + errmsg("trigf: not called by trigger manager")));
>>
>>
>
> Not sure that's appropriate, but I can't see anything else that is
> very appropriate either.
The plpgsql code uses errcode(ERRCODE_FEATURE_NOT_SUPPORTED) for this
situation, so I guess we should be consistent with that.
cheers
andrew
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | David Fetter <david(at)fetter(dot)org>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Kenneth Marshall <ktm(at)rice(dot)edu>, Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-30 02:44:54 |
Message-ID: | 3201.1225334694@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> Not sure that's appropriate, but I can't see anything else that is
>> very appropriate either.
> The plpgsql code uses errcode(ERRCODE_FEATURE_NOT_SUPPORTED) for this
> situation, so I guess we should be consistent with that.
TRIGGERED_DATA_CHANGE_VIOLATION is most certainly NOT an appropriate
code here --- it's talking about invalid database content states.
The RI triggers use ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED for these
sorts of conditions, and I think that's probably best practice. See
ri_CheckTrigger() in particular.
regards, tom lane
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Kenneth Marshall <ktm(at)rice(dot)edu>, Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>, Michael Glaesemann <grzm(at)seespotcode(dot)net>, David Fetter <david(at)fetter(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: minimal update |
Date: | 2008-10-30 03:24:07 |
Message-ID: | 3705.1225337047@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> I think I like this best of all the suggestions -
> suppress_redundant_updates_trigger() is what I have now.
> If there's no further discussion, I'll go ahead and commit this in a day
> or two.
The documentation seems a bit lacking: it gives neither a hint of why
you might want to use this nor why it's not the built-in behavior.
Suggest expending a sentence or two pointing out that the trigger takes
nonzero execution time to do its comparisons, and that this may or may
not be repaid by eliminated updates, depending on whether the client
applications are actually in the habit of issuing useless update
commands.
I think you're missing an <indexentry> item for the function name, also.
regards, tom lane