Doubts about EvalPlanQual

Lists: pgsql-hackers
From: "Jacky Leng" <lengjianquan(at)163(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Doubts about EvalPlanQual
Date: 2009-02-19 05:24:31
Message-ID: gniqee$18cb$1@news.hub.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi,

When I read function "EvalPlanQual", I found the following code:

if (heap_fetch(relation, &SnapshotDirty, &tuple, &buffer, true, NULL))
{
/*
* If xmin isn't what we're expecting, the slot must have been
* recycled and reused for an unrelated tuple. This implies that
* the latest version of the row was deleted, so we need do
* nothing. (Should be safe to examine xmin without getting
* buffer's content lock, since xmin never changes in an existing
* tuple.)
*/
if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
priorXmax))
{
ReleaseBuffer(buffer);
return NULL;
}

AFAICS, when Vacuum decides to reclaim any version V of a tuple T, there
must be none concurrent transactions that are accessing or will access any
versions before V, because HeapTupleSatisfiesVacuum ensures this.

If I'm right, then my doubt is: how can the branch "if
(!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data), priorXmax))"
happen? Is this a dead branch?

If not, can anyone give an example to explain how does this happen?

Thanks a lot.


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Jacky Leng <lengjianquan(at)163(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Doubts about EvalPlanQual
Date: 2009-02-19 08:31:22
Message-ID: 499D18DA.1070100@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Jacky Leng wrote:
> When I read function "EvalPlanQual", I found the following code:
>
> if (heap_fetch(relation, &SnapshotDirty, &tuple, &buffer, true, NULL))
> {
> /*
> * If xmin isn't what we're expecting, the slot must have been
> * recycled and reused for an unrelated tuple. This implies that
> * the latest version of the row was deleted, so we need do
> * nothing. (Should be safe to examine xmin without getting
> * buffer's content lock, since xmin never changes in an existing
> * tuple.)
> */
> if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
> priorXmax))
> {
> ReleaseBuffer(buffer);
> return NULL;
> }
>
> AFAICS, when Vacuum decides to reclaim any version V of a tuple T, there
> must be none concurrent transactions that are accessing or will access any
> versions before V, because HeapTupleSatisfiesVacuum ensures this.
>
> If I'm right, then my doubt is: how can the branch "if
> (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data), priorXmax))"
> happen? Is this a dead branch?
>
> If not, can anyone give an example to explain how does this happen?

Tuples with an aborted xmin can be vacuumed right away. When we're
following the update chain in EvalPlanQual, it's possible that the
updater has aborted, the updated dead tuple is vacuumed away, and the
slot is reused for another unrelated tuple.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: "Jacky Leng" <lengjianquan(at)163(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Doubts about EvalPlanQual
Date: 2009-02-19 10:35:21
Message-ID: gnjcl7$14od$1@news.hub.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> Tuples with an aborted xmin can be vacuumed right away. When we're
> following the update chain in EvalPlanQual, it's possible that the updater
> has aborted, the updated dead tuple is vacuumed away, and the slot is
> reused for another unrelated tuple.

But if the updater aborted, how can EvalPlanQual be called?
In this situation (updater aborted), EvalPlanQual's caller (such as
ExecUpdate and ExecDelete)
should get "HeapTupleMayBeUpdated", rather than "HeapTupleUpdated".

Am I right?


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Jacky Leng <lengjianquan(at)163(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Doubts about EvalPlanQual
Date: 2009-02-19 12:01:24
Message-ID: 499D4A14.4040902@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Jacky Leng wrote:
>> Tuples with an aborted xmin can be vacuumed right away. When we're
>> following the update chain in EvalPlanQual, it's possible that the updater
>> has aborted, the updated dead tuple is vacuumed away, and the slot is
>> reused for another unrelated tuple.
>
> But if the updater aborted, how can EvalPlanQual be called?
> In this situation (updater aborted), EvalPlanQual's caller (such as
> ExecUpdate and ExecDelete)
> should get "HeapTupleMayBeUpdated", rather than "HeapTupleUpdated".

Well, consider this update chain:

A -> B -> C

If A is the tuple visible to the snapshot of the updating query, and the
updater of (A->B) has committed, heap_update/delete call in
ExecUpdate/Delete will return HeapTupleUpdate.

Now that think about this more, I don't see either how that check in
EvalPlanQual could ever be true.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Jacky Leng <lengjianquan(at)163(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Doubts about EvalPlanQual
Date: 2009-02-19 16:01:45
Message-ID: 8519.1235059305@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> Now that think about this more, I don't see either how that check in
> EvalPlanQual could ever be true.

It's there primarily to make the tuple chain chasing code the same
as in other places. Whether it happens to be dead code today because of
arcane details of the usage of EvalPlanQual isn't very interesting.
(I'm unconvinced that it is dead code, but even if it is it'd be folly
to remove it.)

regards, tom lane


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jacky Leng <lengjianquan(at)163(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Doubts about EvalPlanQual
Date: 2009-02-19 16:06:35
Message-ID: 499D838B.3060403@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> (I'm unconvinced that it is dead code, but even if it is it'd be folly
> to remove it.)

Agreed, it's a useful safeguard, even it it's a "can't happen" scenario.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jacky Leng <lengjianquan(at)163(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Doubts about EvalPlanQual
Date: 2009-02-19 22:28:07
Message-ID: 3073cc9b0902191428l7f8fe145p540130010cea79e2@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2/19/09, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> Tom Lane wrote:
>> (I'm unconvinced that it is dead code, but even if it is it'd be folly
>> to remove it.)
>
> Agreed, it's a useful safeguard, even it it's a "can't happen" scenario.
>

if it is a "can't happen" scenario then why not make it an assert?

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Jacky Leng <lengjianquan(at)163(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Doubts about EvalPlanQual
Date: 2009-02-19 22:50:38
Message-ID: 27235.1235083838@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec> writes:
> if it is a "can't happen" scenario then why not make it an assert?

Asserts on data-consistency checks aren't really a good idea.

(IOW this is "can't happen" only as long as your database isn't
corrupt...)

regards, tom lane


From: "Jacky Leng" <lengjianquan(at)163(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Doubts about EvalPlanQual
Date: 2009-02-20 01:31:30
Message-ID: gnl15d$18ka$1@news.hub.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> Asserts on data-consistency checks aren't really a good idea.
>
> (IOW this is "can't happen" only as long as your database isn't
> corrupt...)
>

Then why not change this to an "ereport(PANIC ...)"?


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Jacky Leng <lengjianquan(at)163(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Doubts about EvalPlanQual
Date: 2009-02-20 09:16:00
Message-ID: 499E74D0.5040402@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Jacky Leng wrote:
>> Asserts on data-consistency checks aren't really a good idea.
>>
>> (IOW this is "can't happen" only as long as your database isn't
>> corrupt...)
>>
>
> Then why not change this to an "ereport(PANIC ...)"?

If you have a corrupted database, you want to be able to read it, not
panic. If anything, we could put a WARNING there, but I'm not 100% sure
it really is a "can't happen" case.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com