HeapTupleData.t_self garbage values
in [PgSql]
|
From: Tom Lane on 11 Mar 2010 15:25 "Kevin Grittner" <Kevin.Grittner(a)wicourts.gov> writes: > According to htup.h: > * t_self and t_tableOid should be valid if the HeapTupleData points > * to a disk buffer, or if it represents a copy of a tuple on disk. > * They should be explicitly set invalid in manufactured tuples. > In the heap_hot_search_buffer function of heapam.c this is not true. > I can't find a clear explanation of why that is. I'm assuming "it > just doesn't matter" here, but at a minimum it seems worth a > comment. It's not immediately obvious to me what the random garbage > from the stack would be replaced with if we were to try to make the > above comment true. What that comment is recommending is ItemPointerSetInvalid(&(tuple.t_self)); regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: "Kevin Grittner" on 11 Mar 2010 15:34 Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > "Kevin Grittner" <Kevin.Grittner(a)wicourts.gov> writes: >> According to htup.h: >> * t_self and t_tableOid should be valid if the HeapTupleData >> * points to a disk buffer, or if it represents a copy of a tuple >> * on disk. They should be explicitly set invalid in manufactured >> * tuples. > >> In the heap_hot_search_buffer function of heapam.c this is not >> true. > What that comment is recommending is > > ItemPointerSetInvalid(&(tuple.t_self)); Aren't those tuples pointing to a disk buffer? I know how to set an invalid pointer, but I thought that was only for manufactured tuples? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 11 Mar 2010 15:52 "Kevin Grittner" <Kevin.Grittner(a)wicourts.gov> writes: > Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >> ItemPointerSetInvalid(&(tuple.t_self)); > Aren't those tuples pointing to a disk buffer? Oh, I should have looked at the code before commenting ;-). Yeah, the correct TID value would be ItemPointerGetBlockNumber(tid) plus the current offnum. However we don't have enough information in this function to set t_tableOid correctly, so maybe it would be best to just set both fields invalid. Or do nothing --- AFAICS none of the uses of the heapTuple look at those fields. Is it worth a few extra cycles to initialize unused fields of a short-lived heapTuple? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: "Kevin Grittner" on 11 Mar 2010 16:01 Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Yeah, the correct TID value would be > ItemPointerGetBlockNumber(tid) plus the current offnum. Thanks! > However we don't have enough information in this function to set > t_tableOid correctly, so maybe it would be best to just set both > fields invalid. Or do nothing --- AFAICS none of the uses of the > heapTuple look at those fields. Is it worth a few extra cycles to > initialize unused fields of a short-lived heapTuple? At a minimum, it might be good to qualify the comment in htup.h and add a comment where there is an exception. This can be startling in a debugger if you don't know that the comment isn't really true. (And I've found another place where t_tableOid isn't set, but it is apparently benign; that's without an exhaustive search.) I could put forward a comment-only patch per the above if there are no objections. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 11 Mar 2010 16:12 "Kevin Grittner" <Kevin.Grittner(a)wicourts.gov> writes: > At a minimum, it might be good to qualify the comment in htup.h and > add a comment where there is an exception. This can be startling in > a debugger if you don't know that the comment isn't really true. > (And I've found another place where t_tableOid isn't set, but it is > apparently benign; that's without an exhaustive search.) > I could put forward a comment-only patch per the above if there are > no objections. I don't want to put weasel wording into the comment. If you're bothered by the discrepancy then we should fix the code to initialize all the struct fields. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
|
Next
|
Last
Pages: 1 2 Prev: [HACKERS] HeapTupleData.t_self garbage values Next: buildfarm logging versus embedded nulls |