Thread: Question regarding the database page layout.
<div dir="ltr">Hello all,<br /><br />I have been digging into the database page layout (specifically the tuples) to ensurethe unsigned integer types were consuming the proper storage.<br />While digging around, I found one thing surprising: <br /><br />It appears the heap tuples are padded at the end to the MAXALIGN distance.<br /><br />Below is mydata that I used to come to this conclusion.<br />(This test was performed on a 64-bit system with --with-blocksize=32).<br/><br /> The goal was to compare data from comparable type sizes. <br />The first column indicatesthe type (char, uint1, int2, uint2, int4, and uint4),<br />the number in () indicates the number of columns in thetable. <br /><br />The Length is from the .lp_off field in the ItemId structure.<br /> The Offset is from the .lp_lenfield in the ItemId structure.<br />The Size is the offset difference.<br /><br />char (1) Length Offset Size char (9) Length Offset Size<br /> 25 32736 32 33 32728 40<br /> 25 32704 32 33 32688 40<br /> 25 32672 32 33 32648 40<br /> 25 32640 33 32608 <br /> <br />uint1 (1) Length Offset Size uint1 (9) Length Offset Size<br /> 25 32736 32 33 32728 40<br /> 25 32704 32 33 32688 40<br /> 25 32672 32 33 32648 40<br /> 25 32640 33 32608 <br /> <br />int2 (1) Length Offset Size int2 (5) Length Offset Size<br /> 26 32736 32 34 32728 40<br /> 26 32704 32 34 32688 40<br /> 26 32672 32 34 32648 40<br /> 26 32640 34 32608 <br /> <br />uint2 (1) Length Offset Size unt2 (5) Length Offset Size<br /> 26 32736 32 34 32728 40<br /> 26 32704 32 34 32688 40<br /> 26 32672 32 34 32648 40<br /> 26 32640 34 32608 <br /> <br />int4 (1) Length Offset Size int4 (3) Length Offset Size<br /> 28 32736 32 36 32728 40<br /> 28 32704 32 36 32688 40<br /> 28 32672 32 36 32648 40<br /> 28 32640 36 32608 <br /> <br />uint4 (1) Length Offset Size uint4 (3) Length Offset Size<br /> 28 32736 32 36 32728 40<br /> 28 32704 32 36 32688 40<br /> 28 32672 32 36 32648 40<br /> 28 32640 36 32608 <br /><br />From the documentation at: <a href="http://www.postgresql.org/docs/8.3/static/storage-page-layout.html">http://www.postgresql.org/docs/8.3/static/storage-page-layout.html</a><br />andfrom the comments in src/include/access/htup.h I understand the user data (indicated by t_hoff) <br /> must by a multipleof MAXALIGN distance, but I did not find anything suggesting the heap tuple itself <br />had this requirement.<br/><br />After a cursory glance at the HeapTupleHeaderData structure, it appears it could be aligned with<br/>INTALIGN instead of MAXALIGN. The one structure I was worried about was the 6 byte t_ctid <br />structure. Thecomments in src/include/storage/itemptr.h file indicate the ItemPointerData structure<br /> is composed of 3 int16 fields. So everthing in the HeapTupleHeaderData structure is 32-bits or less.<br /><br />I am interested in attempting togenerate a patch if this idea appears feasible. The current data<br />set I am playing with it would save over 3GB ofdisk space. (Back of the envelope calculations <br /> indicate that 5% of my current storage is consumed by this padding. My tuple length is 44 bytes.)<br /><br />Thanks,<br /><br />- Ryan<br /></div>
"Ryan Bradetich" <rbradetich@gmail.com> writes: > After a cursory glance at the HeapTupleHeaderData structure, it appears it > could be aligned with INTALIGN instead of MAXALIGN. The one structure I was > worried about was the 6 byte t_ctid structure. The comments in > src/include/storage/itemptr.h file indicate the ItemPointerData structure is > composed of 3 int16 fields. So everthing in the HeapTupleHeaderData > structure is 32-bits or less. Sure, but the tuple itself could contain something with double alignment. If you have a bigint or double in the tuple then heap_form_tuple needs to know where to put it so it ends up at right alignment. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support!
Hello Greg,
Thanks,
- Ryan
P.S. Just for the archive, it seems this idea still may be workable (need to look at the
heap_form_tuple in significant more detail) if/when someone implements the proposal
to separate the physical storage from the column order. That solution is a bit more
than I am ready to tackle at the moment :) Maybe in the future.
On Mon, Aug 11, 2008 at 2:24 AM, Gregory Stark <stark@enterprisedb.com> wrote:
My first thought was we can still figure this out because the user data is already
forced to be MAXALIGN. Then I realized oh, since that is true then I am still going
to eat the padding anyhow. The padding would just move to one of two places:
1. To MAXALIGN the user data or
2. To MAXALIGN the heap tuple header.
Thanks for the sanity check. I was wrapped up in looking for alignment requirements
in the HeapTupleHeaderData structure, I overlooked the significance of the MAXALIGN
on the user data.
"Ryan Bradetich" <rbradetich@gmail.com> writes:Sure, but the tuple itself could contain something with double alignment. If
> After a cursory glance at the HeapTupleHeaderData structure, it appears it
> could be aligned with INTALIGN instead of MAXALIGN. The one structure I was
> worried about was the 6 byte t_ctid structure. The comments in
> src/include/storage/itemptr.h file indicate the ItemPointerData structure is
> composed of 3 int16 fields. So everthing in the HeapTupleHeaderData
> structure is 32-bits or less.
you have a bigint or double in the tuple then heap_form_tuple needs to know
where to put it so it ends up at right alignment.
My first thought was we can still figure this out because the user data is already
forced to be MAXALIGN. Then I realized oh, since that is true then I am still going
to eat the padding anyhow. The padding would just move to one of two places:
1. To MAXALIGN the user data or
2. To MAXALIGN the heap tuple header.
Thanks for the sanity check. I was wrapped up in looking for alignment requirements
in the HeapTupleHeaderData structure, I overlooked the significance of the MAXALIGN
on the user data.
Thanks,
- Ryan
P.S. Just for the archive, it seems this idea still may be workable (need to look at the
heap_form_tuple in significant more detail) if/when someone implements the proposal
to separate the physical storage from the column order. That solution is a bit more
than I am ready to tackle at the moment :) Maybe in the future.
Hello all, On Mon, Aug 11, 2008 at 2:24 AM, Gregory Stark <stark@enterprisedb.com> wrote: > > "Ryan Bradetich" <rbradetich@gmail.com> writes: > > > After a cursory glance at the HeapTupleHeaderData structure, it appears it > > could be aligned with INTALIGN instead of MAXALIGN. The one structure I was > > worried about was the 6 byte t_ctid structure. The comments in > > src/include/storage/itemptr.h file indicate the ItemPointerData structure is > > composed of 3 int16 fields. So everthing in the HeapTupleHeaderData > > structure is 32-bits or less. > > Sure, but the tuple itself could contain something with double alignment. If > you have a bigint or double in the tuple then heap_form_tuple needs to know > where to put it so it ends up at right alignment. For fun, I looked around in heap_form_tuple() today to see how big of a job this change would be. It did not seem very hard to implement. I know there are probably several other places I missed with this patch, but this patch does pass the regression tests and simple testing I threw at it (including ALTER TABLE ADD COLUMN, etc). The patch concept is fairly simple. 1. Add a new boolean local variable: require_max_align (initialized to false). 2. When we loop through the attributes, check to see if any attributes require double alignment. If they do, set require_max_align = true. 3. If the tuple has OIDS (double alignment requirement), set require_max_align = true. 4. If require_max_align = true, use the MAXALIGN macro; otherwise use the INTALIGN macro. A check of the physical storage for my previous test case does show the tuple length to be 44 bytes instead of the previous 48 bytes, so the patch does appear to work. I have not run any performance tests using this patch, etc. Attached is my proof-of-concept patch for this idea in case someone wants to inform me of other problems this patch will introduce :) Thanks, - Ryan
Attachment
On Tue, Sep 02, 2008 at 01:49:43AM -0700, Ryan Bradetich wrote: > For fun, I looked around in heap_form_tuple() today to see how big of a job this > change would be. It did not seem very hard to implement. I know there are > probably several other places I missed with this patch, but this patch does pass > the regression tests and simple testing I threw at it (including ALTER TABLE > ADD COLUMN, etc). You need to arrange testing on an architechture that has strict alignment reuiqrements. For example i386 doesn't care about alignment at all and will anything from anywhere, with performance degradation. Other architechtures will simply throw exceptions, that's the smoke test. Also, what's the performance cost? Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines.
"Ryan Bradetich" <rbradetich@gmail.com> writes: > The patch concept is fairly simple. > 1. Add a new boolean local variable: require_max_align > (initialized to false). This really can't possibly work, because you'd need to propagate knowledge of the tuple's alignment requirement all over the place. In particular, how would code *reading* the tuple know where the data starts? Also, I don't think you get (very much of) the actual benefit unless the code that inserts tuples into disk pages knows to do something different in the int-align case. It's conceivable that we could make this work if we wanted to dedicate an infomask bit to showing whether the tuple needs int or double alignment. I don't really think it's worth the trouble though. regards, tom lane
"Ryan Bradetich" <rbradetich@gmail.com> writes: > 4. If require_max_align = true, use the MAXALIGN macro; otherwise > use the INTALIGN macro. Huh, I didn't think of doing it like that. But I'm confused. You seem to be tweaking the alignment of the data inside the tuple? After the tuple header? I thought we had only one byte of wasted space in there and that's used by the null bitmap. So unless you have more than 8 columns and some of them are null I wouldn't expect you to save any space. If you do then I guess you could save 4 bytes if the null bitmap is 2-5 bytes (mod 8) long. I thought the goal was to save space by aligning the tuples on the page more densely. That seems to me to be more fruitful as about half the tuples will save four bytes even on tables with small or missing null bitmaps. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support!
Tom Lane <tgl@sss.pgh.pa.us> writes: > "Ryan Bradetich" <rbradetich@gmail.com> writes: >> The patch concept is fairly simple. > >> 1. Add a new boolean local variable: require_max_align >> (initialized to false). > > This really can't possibly work, because you'd need to propagate > knowledge of the tuple's alignment requirement all over the place. > In particular, how would code *reading* the tuple know where the > data starts? Uh, at t_hoff, no? > Also, I don't think you get (very much of) the actual benefit unless the > code that inserts tuples into disk pages knows to do something different in > the int-align case. +1 -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's PostGIS support!
Hello Martijn, > You need to arrange testing on an architechture that has strict > alignment reuiqrements. For example i386 doesn't care about alignment > at all and will anything from anywhere, with performance degradation. > > Other architechtures will simply throw exceptions, that's the smoke > test. From other comments in this thread, I still have quite a bit of work ahead of me :) If I can accomplish this, then I will run it on a different architecture. I have access to a parisc-linux and a powerpc system. > Also, what's the performance cost? Not sure yet. My goal would be 0 impact on 32-bit system and something small on 64-bit systems that would hopefully be overcome by a performance gain in IO. Thanks for your feedback. - Ryan
Hello Tom, On Tue, Sep 2, 2008 at 8:07 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Ryan Bradetich" <rbradetich@gmail.com> writes: >> The patch concept is fairly simple. > >> 1. Add a new boolean local variable: require_max_align >> (initialized to false). > > This really can't possibly work, because you'd need to propagate > knowledge of the tuple's alignment requirement all over the place. > In particular, how would code *reading* the tuple know where the > data starts? Also, I don't think you get (very much of) the actual > benefit unless the code that inserts tuples into disk pages knows > to do something different in the int-align case. I figured there was something wrong with my proof-of-concept patch because it was too easy and it passed regression tests on the first attempt. I just didn't know what was wrong. After reading your comment, I went back and looked at my test data and I am embarrassed to admit that I misread the data :( Sure the tuple length is 44 bytes, but the data is still aligned on a 8 byte boundary. So this poc patch effectively does nothing :( As Greg mentioned, the code reading the user data would know where to start because of the t_hoff field. In my first email in this thread, I had looked at the requirements for the HeapTupleHeaderData to be MAXALIGN. I could not find any reason the HeapTupleHeaderData needed to be MAXALIGN, but Greg pointed out that the t_hoff filed is required to be MAXALIGN. I can only see two reasons why the t_hoff field is required to be MAXALIGN: 1. If the tuple has oids (OIDS require MAXALIGN). 2. Ifthe tuple has any column requiring MAXALIGN. If neither of the above conditions are true, then it seems to me that t_hoff could safely be INTALIGN. I was working under the mistaken assumption that if I addressed this issue in heap_form_tuple, that it would be written down on disk this way. Obviously I was wrong and still have a lot of code reading and experimentation to do :) > It's conceivable that we could make this work if we wanted to dedicate > an infomask bit to showing whether the tuple needs int or double > alignment. I don't really think it's worth the trouble though. I am not sure what an infomask bit is yet, but is it safe to assume (in theory) that if I get the page written correctly, that the tuple reading code would work because of the t_hoff field? Thanks for your feedback! - Ryan
Hello Greg, On Tue, Sep 2, 2008 at 8:30 AM, Gregory Stark <stark@enterprisedb.com> wrote: > "Ryan Bradetich" <rbradetich@gmail.com> writes: > >> 4. If require_max_align = true, use the MAXALIGN macro; otherwise >> use the INTALIGN macro. > > Huh, I didn't think of doing it like that. > > But I'm confused. You seem to be tweaking the alignment of the data inside the > tuple? After the tuple header? I thought we had only one byte of wasted space > in there and that's used by the null bitmap. So unless you have more than 8 > columns and some of them are null I wouldn't expect you to save any space. If > you do then I guess you could save 4 bytes if the null bitmap is 2-5 bytes > (mod 8) long. I was not trying to tweak the alignment of the data inside the tuple header, I was trying to adjust the alignment of t_hoff so it would not have the requirement of MAXALIGN. I believe my proof-of-concept patch was bad and I need to spend some more time on it tonight with the new knowledge I gained from this email thread. > I thought the goal was to save space by aligning the tuples on the page more > densely. That seems to me to be more fruitful as about half the tuples will > save four bytes even on tables with small or missing null bitmaps. That is the goal. Basically my test data is 44 bytes in length for each tuple. I have no data in the tuple that is required to be MAXALIGN, but since t_hoff has the MAXALIGN requirement I throw away 4 bytes for each tuple (on a 64-bit machine). This proof-of-concept patch is to help me understand the PostgreSQL code and to see if I can recover those 4 bytes per tuple. Thanks again for your feedback! - Ryan
Gregory Stark <stark@enterprisedb.com> writes: > Tom Lane <tgl@sss.pgh.pa.us> writes: >> In particular, how would code *reading* the tuple know where the >> data starts? > Uh, at t_hoff, no? Doh, right. Obviously need more caffeine. regards, tom lane
"Ryan Bradetich" <rbradetich@gmail.com> writes: > Hello Greg, > > On Tue, Sep 2, 2008 at 8:30 AM, Gregory Stark <stark@enterprisedb.com> wrote: >> >> But I'm confused. You seem to be tweaking the alignment of the data inside the >> tuple? After the tuple header? I thought we had only one byte of wasted space >> in there and that's used by the null bitmap. So unless you have more than 8 >> columns and some of them are null I wouldn't expect you to save any space. If >> you do then I guess you could save 4 bytes if the null bitmap is 2-5 bytes >> (mod 8) long. > > I was not trying to tweak the alignment of the data inside the tuple header, > I was trying to adjust the alignment of t_hoff so it would not have the > requirement of MAXALIGN. I believe my proof-of-concept patch was bad and I > need to spend some more time on it tonight with the new knowledge I gained > from this email thread. The structure on the page is to have a bunch of tuples stored one after the other. Each tuple is maxaligned after the previous (actually before the previous since they start from the end but we'll ignore that -- the point is that they all *start* at a maxaligned position on the page). Within each tuple there's a header which is 23 bytes long. Then an optional null bitmap. In the header is t_hoff which points to the first byte of actual data -- effectively specifying the length of the null bitmap. t_hoff is maxaligned as well. So by intaligning t_hoff you're adjusting where the data within the tuple fits by making the null bitmap 4 bytes shorter. That seems worth a few cycles but isn't going to affect every table. It'll only affect tables that have a null bitmap between 2-5 bytes (mod 8), ie, with 9-40 fields at least one of which is null. Tables with 9 fields are likely to be relatively wide which means saving 4 bytes isn't that much of a gain percentagewise. (Though they might be mostly null....) The other 4 bytes you could save is by packing the whole tuples themselves more closely on the page. That happens when the item pointer is added and pointed to the tuple. To do that heap_form_tuple would have to return data back to the caller about the alignment needed to whomever is going to copy it into the page. AFAICS that could be done in the HeapTuple structure itself rather than in the tuple though which doesn't seem so bad. Tom may be seeing something I'm not though. Incidentally, can't you set the alignment during the compute_data_size() call instead of doing an extra pass through the attributes? Probably doesn't change the cost though. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services!
"Ryan Bradetich" <rbradetich@gmail.com> writes: > On Tue, Sep 2, 2008 at 8:07 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> It's conceivable that we could make this work if we wanted to dedicate >> an infomask bit to showing whether the tuple needs int or double >> alignment. I don't really think it's worth the trouble though. > I am not sure what an infomask bit is yet, but is it safe to assume > (in theory) that if I get the > page written correctly, that the tuple reading code would work because > of the t_hoff field? Well, as Greg pointed out, setting t_hoff correctly should be sufficient for squeezing out useless padding between the tuple header and the tuple data. The real objection here is that that's leaving most of the possible gain still on the table. The tuple *as a whole* (header and data both) is still going to get maxaligned when it's plopped onto a disk page. I think that in real-world cases that's often where the main padding cost comes in: with 23-byte tuple headers and no OIDs, you aren't saving a thing by int-aligning t_hoff unless you have a nulls bitmap and it's the wrong number of bytes long. So IMHO the real problem is over near PageAddItem. regards, tom lane
Gregory Stark <stark@enterprisedb.com> writes: > The other 4 bytes you could save is by packing the whole tuples themselves > more closely on the page. That happens when the item pointer is added and > pointed to the tuple. To do that heap_form_tuple would have to return data > back to the caller about the alignment needed to whomever is going to copy it > into the page. AFAICS that could be done in the HeapTuple structure itself > rather than in the tuple though which doesn't seem so bad. Tom may be seeing > something I'm not though. You'd need to keep the info available for subsequent re-compactions of the page, so I think an infomask bit is really the only solution that would work. But on reflection, that's really a separable issue from whether we minimize t_hoff padding. And as Ryan's patch shows, it's not that hard to do the latter. We should probably grab the low-hanging fruit here. > Incidentally, can't you set the alignment during the compute_data_size() call > instead of doing an extra pass through the attributes? +1. This code is executed often enough that every little bit helps. BTW, there are at least two copies of that code to be changed. I'd suggest grepping for assignments to t_hoff to be sure there aren't more. regards, tom lane
Hello Greg, On Tue, Sep 2, 2008 at 9:54 AM, Gregory Stark <stark@enterprisedb.com> wrote: > "Ryan Bradetich" <rbradetich@gmail.com> writes: > The structure on the page is to have a bunch of tuples stored one after the > other. Each tuple is maxaligned after the previous (actually before the > previous since they start from the end but we'll ignore that -- the point is > that they all *start* at a maxaligned position on the page). > > Within each tuple there's a header which is 23 bytes long. Then an optional > null bitmap. In the header is t_hoff which points to the first byte of actual > data -- effectively specifying the length of the null bitmap. t_hoff is > maxaligned as well. Thanks for your persistence in explaining this issue to me. I think I am starting to understand the issues now and I appreciate the help. I think I see the error in my thinking. You and Tom are saying the tuple is MAXALIGN on the page as well. My assumption was that since t_hoff had to be MAXALIGN, the tuple on the page was being MAXALIGN by default. If I was able to INTALIGNt_hoff then the tuple on the page would be free to be INTALIGN as well. Tom gave me the next place to go look later tonight (PageAddItem). > So by intaligning t_hoff you're adjusting where the data within the tuple fits > by making the null bitmap 4 bytes shorter. That seems worth a few cycles but > isn't going to affect every table. It'll only affect tables that have a null > bitmap between 2-5 bytes (mod 8), ie, with 9-40 fields at least one of which > is null. Tables with 9 fields are likely to be relatively wide which means > saving 4 bytes isn't that much of a gain percentagewise. (Though they might be > mostly null....) > > The other 4 bytes you could save is by packing the whole tuples themselves > more closely on the page. That happens when the item pointer is added and > pointed to the tuple. To do that heap_form_tuple would have to return data > back to the caller about the alignment needed to whomever is going to copy it > into the page. AFAICS that could be done in the HeapTuple structure itself > rather than in the tuple though which doesn't seem so bad. Tom may be seeing > something I'm not though. > > Incidentally, can't you set the alignment during the compute_data_size() call > instead of doing an extra pass through the attributes? Probably doesn't change > the cost though. Actually I did not do an extra pass through the attributes. I added the check in an existing pass through the attributes. I did look at adding this check to the heap_compute_data_size(), but took the easy path to see if my patch worked. I will update my patch to do the alignment check in heap_compute_data_size(). Thanks! - Ryan
Hello Tom, > Well, as Greg pointed out, setting t_hoff correctly should be sufficient > for squeezing out useless padding between the tuple header and the tuple > data. The real objection here is that that's leaving most of the > possible gain still on the table. The tuple *as a whole* (header and > data both) is still going to get maxaligned when it's plopped onto a > disk page. I think that in real-world cases that's often where the > main padding cost comes in: with 23-byte tuple headers and no OIDs, > you aren't saving a thing by int-aligning t_hoff unless you have a nulls > bitmap and it's the wrong number of bytes long. > > So IMHO the real problem is over near PageAddItem. Thanks for your feed back as well. Between you and Greg I think I finally understand the error in my thinking. I will investigate the PageAddItem() later tonight. - Ryan
Tom Lane <tgl@sss.pgh.pa.us> writes: > BTW, there are at least two copies of that code to be changed. I'd > suggest grepping for assignments to t_hoff to be sure there aren't more. I did send in a patch a while ago to get rid of the old HeapFormTuple() and friends. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB'sPostgreSQL training!
Gregory Stark <stark@enterprisedb.com> writes: > Tom Lane <tgl@sss.pgh.pa.us> writes: >> BTW, there are at least two copies of that code to be changed. I'd >> suggest grepping for assignments to t_hoff to be sure there aren't more. > I did send in a patch a while ago to get rid of the old HeapFormTuple() and > friends. I remember discussing that idea, but I don't recall seeing an actual patch? It would have to be quite large because of the number of places using the old way. I'd also be a bit worried about breaking add-on modules to little purpose ... regards, tom lane
Tom Lane escribió: > Gregory Stark <stark@enterprisedb.com> writes: > > Tom Lane <tgl@sss.pgh.pa.us> writes: > >> BTW, there are at least two copies of that code to be changed. I'd > >> suggest grepping for assignments to t_hoff to be sure there aren't more. Besides heap_form_tuple and heap_formtuple, we have heap_form_minimal_tuple head_addheader (it's also assigned to in heap_xlog_insert and heap_xlog_update, but I'm not sure they need anything changed) > > I did send in a patch a while ago to get rid of the old HeapFormTuple() and > > friends. > > I remember discussing that idea, but I don't recall seeing an actual > patch? It would have to be quite large because of the number of places > using the old way. I'd also be a bit worried about breaking add-on > modules to little purpose ... The good news is that it's easy to convert each caller (of which there are 57 currently BTW) to the new form; and we could just have heap_formtuple be a wrapper over heap_form_tuple, to reduce breakage. Maybe add a #warning about deprecation in there for a release or two. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Tom Lane <tgl@sss.pgh.pa.us> writes: > Gregory Stark <stark@enterprisedb.com> writes: >> Tom Lane <tgl@sss.pgh.pa.us> writes: >>> BTW, there are at least two copies of that code to be changed. I'd >>> suggest grepping for assignments to t_hoff to be sure there aren't more. > >> I did send in a patch a while ago to get rid of the old HeapFormTuple() and >> friends. > > I remember discussing that idea, but I don't recall seeing an actual > patch? It would have to be quite large because of the number of places > using the old way. I'd also be a bit worried about breaking add-on > modules to little purpose ... Huh, apparently I did it but didn't actually send in the patch: http://archives.postgresql.org/pgsql-hackers/2007-10/msg00851.php I looked around and I don't seem to have it lying around any more. (Kind of mystifying since I have tons of old source trees and patches, just not that one.) I could do the janitorial work again if we're interested. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning
Gregory Stark <stark@enterprisedb.com> writes: > I could do the janitorial work again if we're interested. I think it'd make more sense to do it incrementally rather than in one big-bang patch ... regards, tom lane