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

On 2014-04-27 17:49:53 -0400, Tom Lane wrote:
> Andres Freund <andres@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 be used), and
> >> the second one by exploding the ROW() order-by into separate order-by
> >> columns.
> 
> > The problem is that - at least to my knowledge - it's not possible to do
> > the WHERE part as an indexable clause using individual columns.
> 
> You mean like this?
> 
> ..
> The code for extracting prefixes of RowCompareExprs like that has existed
> for quite some time.  But it doesn't understand about whole-row variables.

Yea, but:

> 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, but now we do.

e.g.
set enable_seqscan = false; set enable_bitmapscan = false;
EXPLAIN (ANALYZE, BUFFERS)
SELECT oid
FROM pg_rewrite
WHERE ROW(ev_class, rulename, ev_action) >= ROW('pg_user_mappings'::regclass, '_RETURN',NULL)
ORDER BY ROW(ev_class, rulename, ev_action);

goes from 0.527ms to 9.698ms. And that's damn few rows.

> >> On the whole I feel fairly good about the opinion that this change won't
> >> be disastrous for mainstream usages, and will be beneficial for
> >> performance some of the time.
> 
> > I am less convinced of that. But I don't have a better idea. How about
> > letting it stew in HEAD for a while? It's not like it's affecting all
> > that many people, given the amount of reports over the last couple
> > years.
> 
> Well, mumble.  It's certainly true that it took a long time for someone to
> produce a reproducible test case.  But it's not like we don't regularly
> hear reports of corrupted data with "missing chunk number 0 for toast
> value ...".  Are you really going to argue that few of those reports can
> be blamed on this class of bug?  If so, on what evidence?

No, I am not claiming that.

> Of course
> I have no evidence either to claim that this *is* biting people; we don't
> know, since it never previously occurred to us to ask complainants if they
> were using arrays-of-composites or one of the other risk cases.  But it
> seems to me that letting a known data-loss bug go unfixed on the grounds
> that the fix might create performance issues for some people is not a
> prudent way to proceed.  People expect a database to store their data
> reliably, period.

I agree that this needs to get backpatched at some point. But people
also get very upset if queries gets slower by a magnitude or two after a
minor version upgrade. And this does have the potential to do that, no?

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services