Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-28 Thread Andres Freund
On 2014-04-27 18:44:16 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: Just for some clarity, that also happens with expressions like: WHERE ROW(ev_class, rulename, ev_action) = ROW('pg_rewrite'::regclass, '_RETURN', NULL) ORDER BY ROW(ev_class, rulename,

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-28 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2014-04-27 18:44:16 -0400, Tom Lane wrote: They don't get nearly as upset as they do if the database loses their data. Yes, in an ideal world, we could fix this and not suffer any performance loss anywhere. But no such option is on the table,

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-27 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2014-04-25 12:05:17 -0400, Tom Lane wrote: Meh ... is it likely that the columns involved in an ordering comparison would be so wide as to be toasted out-of-line? Such a query would only be fast if the row value were indexed, which would pretty

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-27 Thread Andres Freund
On 2014-04-27 14:18:46 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-04-25 12:05:17 -0400, Tom Lane wrote: Meh ... is it likely that the columns involved in an ordering comparison would be so wide as to be toasted out-of-line? Such a query would only be fast

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-27 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2014-04-27 14:18:46 -0400, Tom Lane wrote: Ah, I see. Well, we're pretty darn stupid about such queries anyway :-(. Your first example could be greatly improved by expanding the whole-row Var into a ROW() construct (so that RowCompareExpr could

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-27 Thread Andres Freund
On 2014-04-27 17:49:53 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-04-27 14:18:46 -0400, Tom Lane wrote: Ah, I see. Well, we're pretty darn stupid about such queries anyway :-(. Your first example could be greatly improved by expanding the whole-row Var

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-27 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: Just for some clarity, that also happens with expressions like: WHERE ROW(ev_class, rulename, ev_action) = ROW('pg_rewrite'::regclass, '_RETURN', NULL) ORDER BY ROW(ev_class, rulename, ev_action); Previously we wouldn't detoast ev_action here,

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-25 Thread Heikki Linnakangas
On 04/25/2014 02:40 AM, Tom Lane wrote: * The patch changes HeapTupleGetDatum from a simple inline macro into a function call. This means that third-party extensions will not get protection against creation of toast-pointer-containing composite Datums until they recompile. One consequence of

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-25 Thread Tom Lane
Heikki Linnakangas hlinnakan...@vmware.com writes: On 04/25/2014 02:40 AM, Tom Lane wrote: * The patch changes HeapTupleGetDatum from a simple inline macro into a function call. This means that third-party extensions will not get protection against creation of toast-pointer-containing

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-25 Thread Andres Freund
Hi, On 2014-04-24 19:40:30 -0400, Tom Lane wrote: * Because HeapTupleGetDatum might allocate a new tuple, the wrong thing might happen if the caller changes CurrentMemoryContext between heap_form_tuple and HeapTupleGetDatum. It's fscking ugly to allocate memory in a PG_RETURN_... But I don't

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-25 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2014-04-24 19:40:30 -0400, Tom Lane wrote: * Because HeapTupleGetDatum might allocate a new tuple, the wrong thing might happen if the caller changes CurrentMemoryContext between heap_form_tuple and HeapTupleGetDatum. It's fscking ugly to

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-25 Thread Andres Freund
On 2014-04-25 11:22:09 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-04-24 19:40:30 -0400, Tom Lane wrote: * Because HeapTupleGetDatum might allocate a new tuple, the wrong thing might happen if the caller changes CurrentMemoryContext between heap_form_tuple

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-25 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: The case I am worried most about is queries like: SELECT a, b FROM f WHERE f ROW(38, 'whatever') ORDER BY f; I've seen such generated by a some query generators for paging. But I guess that's something we're going to have to accept. Meh ... is it

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-25 Thread Andres Freund
On 2014-04-25 12:05:17 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: The case I am worried most about is queries like: SELECT a, b FROM f WHERE f ROW(38, 'whatever') ORDER BY f; I've seen such generated by a some query generators for paging. But I guess that's

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-25 Thread Andres Freund
On 2014-04-25 18:25:44 +0200, Andres Freund wrote: On 2014-04-25 12:05:17 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: The case I am worried most about is queries like: SELECT a, b FROM f WHERE f ROW(38, 'whatever') ORDER BY f; I've seen such generated by a some

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-24 Thread Tom Lane
I wrote: I'm actually planning to set this patch on the shelf for a bit and go investigate the other alternative, ie, not generating composite Datums containing toast pointers in the first place. Here's a draft patch along those lines. It turned out to be best to leave heap_form_tuple() alone

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-22 Thread Tom Lane
Noah Misch n...@leadboat.com writes: On Mon, Apr 21, 2014 at 10:57:34AM -0400, Tom Lane wrote: I'm actually planning to set this patch on the shelf for a bit and go investigate the other alternative, ie, not generating composite Datums containing toast pointers in the first place. I maintain

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-21 Thread Tom Lane
Noah Misch n...@leadboat.com writes: I wonder how it would work out to instead delay this new detoast effort until toast_insert_or_update(). That would require toast_insert_or_update() to know about every container datatype. I doubt it could lead to an extensible or maintainable solution. I'm

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-21 Thread Andres Freund
Hi, On 2014-04-20 15:38:23 -0400, Tom Lane wrote: Some poking around with oprofile says that much of the added time is coming from added syscache and typcache lookups, as I suspected. I did some further work on the patch to make it possible to cache the element tuple descriptor across calls

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-21 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: I unfortunately haven't followed this in detail, but shouldn't it be relatively easily to make this even cheaper by checking for HEAP_HASEXTERNAL? If it's not set we don't need to iterate over the composite's columns, right? That's the point I

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-21 Thread Andres Freund
On 2014-04-21 11:30:57 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: I unfortunately haven't followed this in detail, but shouldn't it be relatively easily to make this even cheaper by checking for HEAP_HASEXTERNAL? If it's not set we don't need to iterate over the

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-21 Thread Merlin Moncure
On Mon, Apr 21, 2014 at 10:40 AM, Andres Freund and...@2ndquadrant.com wrote: On 2014-04-21 11:30:57 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: I unfortunately haven't followed this in detail, but shouldn't it be relatively easily to make this even cheaper by

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-21 Thread Alvaro Herrera
Merlin Moncure wrote: On Mon, Apr 21, 2014 at 10:40 AM, Andres Freund and...@2ndquadrant.com wrote: And too bad that infomask bits are so scarce :(. We really need to reclaim HEAP_MOVED_OFF and HEAP_MOVED_IN. The only consequence of that is losing support for in-place update for

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-21 Thread Noah Misch
On Mon, Apr 21, 2014 at 10:57:34AM -0400, Tom Lane wrote: Noah Misch n...@leadboat.com writes: I wonder how it would work out to instead delay this new detoast effort until toast_insert_or_update(). That would require toast_insert_or_update() to know about every container datatype. I

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-20 Thread Tom Lane
I wrote: The main problem with this patch, as I see it, is that it'll introduce extra syscache lookup overhead even when there are no toasted fields anywhere. I've not really tried to quantify how much, since that would require first agreeing on a benchmark case --- anybody have a thought

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-20 Thread Noah Misch
I lack time to give this a solid review, but here's a preliminary reaction: On Sun, Apr 20, 2014 at 03:38:23PM -0400, Tom Lane wrote: On HEAD, this takes about 295-300 msec on my machine in a non-cassert build. With the patch I sent previously, the time goes to 495-500 msec. This goes from

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-04-18 Thread Tom Lane
Noah Misch n...@leadboat.com writes: 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.

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-03-29 Thread Noah Misch
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

[HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-03-28 Thread Tom Lane
Way way back in commit ae93e5fd6e8a7e2321e87d23165d9d7660cde598, we established a coding rule that it was okay for composite Datums to contain external (out-of-line) toasted fields, as long as such toasting didn't go more than one level deep in any tuple. This meant that heap_form_tuple had to go

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-03-28 Thread Merlin Moncure
On Fri, Mar 28, 2014 at 3:34 PM, Tom Lane t...@sss.pgh.pa.us wrote: Way way back in commit ae93e5fd6e8a7e2321e87d23165d9d7660cde598, we established a coding rule that it was okay for composite Datums to contain external (out-of-line) toasted fields, as long as such toasting didn't go more than

Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)

2014-03-28 Thread Tom Lane
Merlin Moncure mmonc...@gmail.com writes: Trying to follow along here. Am I correct in saying that if you make these changes any SQL level functionality (say, creating a composite type containing a large array) that used to work will continue to work? This wouldn't change any SQL-level