Interesting bug. On Fri, Mar 28, 2014 at 04:34:41PM -0400, Tom Lane wrote: > I think we might be better off to get rid of toast_flatten_tuple_attribute > and instead insist that composite Datums never contain any external toast > pointers in the first place. That is, places that call heap_form_tuple > to make a composite Datum (rather than a tuple that's due to be stored > to disk) would be responsible for detoasting any fields with external > values first. We could make a wrapper routine for heap_form_tuple to > take care of this rather than duplicating the code in lots of places. > > >From a performance standpoint this is probably a small win. In cases > where a composite Datum is formed and ultimately saved to disk, it should > be a win, since we'd have to detoast those fields anyway, and we can avoid > the overhead of an extra disassembly and reassembly of the composite > value. If the composite value is never sent to disk, it's a bit harder > to tell: we lose if the specific field value is never extracted from the > composite, but on the other hand we win if it's extracted more than once.
Performance is the dominant issue here; the hacker-centric considerations you outlined vanish next to it. Adding a speculative detoast can regress by a million-fold the performance of a query that passes around, without ever dereferencing, toast pointers to max-size values. Passing around a record without consulting all fields is a credible thing to do, so I'd scarcely consider taking the performance risk of more-aggressively detoasting every composite. That other cases win is little comfort. Today's PostgreSQL applications either suffer little enough to care or have already taken measures to avoid duplicate detoasts. Past discussions have examined general ways to avoid repetitive detoast traffic; we'll have something good when it can do that without imposing open-ended penalties on another usage pattern. Making the array construction functions use toast_flatten_tuple_attribute() carries a related performance risk, more elusive yet just as costly when encountered. That much risk seems tolerable for 9.4, though. I won't worry about performance regressions for range types; using a range to marshal huge values you'll never detoast is a stretch. > In any case, adding the extra syscache lookups involved in doing > toast_flatten_tuple_attribute in lots more places isn't appealing. True; alas. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
