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

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Noah Misch <noah(at)leadboat(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Composite Datums containing toasted fields are a bad idea(?)
Date: 2014-04-27 22:10:43
Message-ID: 20140427221043.GB2815@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2014-04-27 17:49:53 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2014-04-27 22:44:16 Re: Composite Datums containing toasted fields are a bad idea(?)
Previous Message Tom Lane 2014-04-27 21:49:53 Re: Composite Datums containing toasted fields are a bad idea(?)