Re: BUG #5798: Some weird error with pl/pgsql procedure

From: Maxim Boguk <maxim(dot)boguk(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-bugs(at)postgresql(dot)org
Subject: Re: BUG #5798: Some weird error with pl/pgsql procedure
Date: 2011-02-21 10:58:38
Message-ID: AANLkTi=H3wOXQ+vx7mJEVkw5o=GcRiLbyTWBmUKLwb0b@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Hi

Yes here is one BEFORE UPDATE trigger on relation
(
Triggers:
_billing_main_denyaccess_71 BEFORE INSERT OR DELETE OR UPDATE ON
ctrl_servers FOR EACH ROW EXECUTE PROCEDURE
_billing_main.denyaccess('_billing_main')
)

However that trigger should not fire at all because storable procedure work
with set local session_replication_role to 'replica';

But tuple become "virtual" in these lines:

2027 newtuple = ExecBRUpdateTriggers(estate,
resultRelInfo,

2028
tupleid, tuple);
2029
2030 if (newtuple == NULL) /* "do nothing" */
2031 return;
2032
2033 if (newtuple != tuple) /* modified by Trigger(s) */
2034 {
2035 /*
2036 * Put the modified tuple into a slot for
convenience of routines
2037 * below. We assume the tuple was allocated
in per-tuple memory
2038 * context, and therefore will go away by
itself. The tuple table
2039 * slot should not try to clear it.
2040 */
2041 TupleTableSlot *newslot =
estate->es_trig_tuple_slot;
2042
2043 if (newslot->tts_tupleDescriptor !=
slot->tts_tupleDescriptor)
2044 ExecSetSlotDescriptor(newslot,
slot->tts_tupleDescriptor);
2045 ExecStoreTuple(newtuple, newslot,
InvalidBuffer, false);
2046 slot = newslot;
2047 tuple = newtuple;
2048 }

If be more exact at line:
2047 tuple = newtuple;

What I don't understand fist it is why that code path fail only once per
1000-100000 calls only.

I have an idea that
2033 if (newtuple != tuple) /* modified by Trigger(s) */
sometime can return true even if no triggers really had been done.

After more code digging I think I found reason
Look like that happens in this part of ExecBRUpdateTriggers code:

02273 /*
02274 * In READ COMMITTED isolation level it's possible that newtuple was
02275 * changed due to concurrent update. In that case we have a
raw subplan
02276 * output tuple and need to run it through the junk filter.
02277 */
02278 if (newSlot != NULL
<http://doxygen.postgresql.org/c_8h.html#070d2ce7b6bb7e5c05602aa8c308d0c4>)
02279 intuple = newtuple = ExecRemoveJunk
<http://doxygen.postgresql.org/execJunk_8c.html#9779200174530a361ea70e21639c2c72>(relinfo->ri_junkFilter
<http://doxygen.postgresql.org/structResultRelInfo.html#0d887ebe5510dd3e663e95888d72cd67>,
newSlot);

That rarity of that situation become understandable (that is very very rare
case when tuple got changed in such way) and really create virtual tuple.

Look like need put tuple materialization code here as well.

PS: again may be just handwaving... I just started learn postgresql code.
Any help from more skilled persons would be great.

On Mon, Feb 21, 2011 at 10:07 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Maxim Boguk <maxim(dot)boguk(at)gmail(dot)com> writes:
> > I tracked virtual tuple from heaptuple.c::slot_getattr down to
> > execMain.c::ExecUpdate and I think somewhere in this way virtual tuple
> > should be need to be "materialized" into physical tuple.
>
> That happens at the top of ExecUpdate:
>
> /*
> * get the heap tuple out of the tuple table slot, making sure we have a
> * writable copy
> */
> tuple = ExecMaterializeSlot(slot);
>
> I don't see any code path there that could re-virtualize the slot.
>
> Do you have any triggers or constraints on the target table? Because
> those are about the only things that influence this code path at all...
>
> > Again I not sure but ExecProcessReturning function seems good candidate
> to
> > perform tuple materialization.
>
> If ExecProcessReturning were given a virtual slot, materializing it
> would do no good at all for this problem, because the system columns in
> the result would be garbage. It has to receive an already materialized
> slot that contains the right information beforehand. The only reason
> that retrieving ctid can work at all is that when heap_update is called,
> "tuple" is pointing at the slot's contained tuple and so setting
> tuple->t_self changes the contents of the slot. That whole function is
> completely dependent on the slot contents being physical not virtual,
> and I don't see anything there that would break it.
>
> regards, tom lane
>

--
Maxim Boguk
Senior Postgresql DBA.

Skype: maxim.boguk
Jabber: maxim(dot)boguk(at)gmail(dot)com

LinkedIn profile: http://nz.linkedin.com/in/maximboguk
If they can send one man to the moon... why can't they send them all?

МойКруг: http://mboguk.moikrug.ru/
Сила солому ломит, но не все в нашей жизни - солома, да и сила далеко не
все.

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Nacho Mezzadra 2011-02-21 15:54:48 BUG #5896: When server cannot be started, first it says that it couldn't be started and then Server Started
Previous Message Fabien COELHO 2011-02-21 08:28:15 Re: [BUGS] issue about information_schema REFERENTIAL_CONSTRAINTS