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

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?

regression=# EXPLAIN verbose SELECT * FROM pg_rewrite r WHERE r > ('x'::name, '11854'::oid, NULL, NULL, NULL, NULL,
null);                                                     QUERY PLAN
    
 

-------------------------------------------------------------------------------------------------------------------------Seq
Scanon pg_catalog.pg_rewrite r  (cost=0.00..46.21 rows=59 width=983)  Output: rulename, ev_class, ev_type, ev_enabled,
is_instead,ev_qual, ev_action  Filter: (r.* > ROW('x'::name, 11854::oid, NULL::unknown, NULL::unknown, NULL::unknown,
NULL::unknown,NULL::unknown))Planning time: 0.119 ms
 
(4 rows)

regression=# EXPLAIN verbose SELECT * FROM pg_rewrite r WHERE row(rulename, ev_class, ev_type, ev_enabled, is_instead,
ev_qual,ev_action) > ('x'::name, '11854'::oid, NULL, NULL, NULL, NULL, null);
                                                          QUERY PLAN
                                                  
 

------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------Index
Scanusing pg_rewrite_rel_rulename_index on pg_catalog.pg_rewrite r  (cost=0.15..13.50 rows=1 width=983)  Output:
rulename,ev_class, ev_type, ev_enabled, is_instead, ev_qual, ev_action  Index Cond: (ROW(r.rulename, r.ev_class) >=
ROW('x'::name,11854::oid))  Filter: (ROW(r.rulename, r.ev_class, r.ev_type, r.ev_enabled, r.is_instead,
(r.ev_qual)::text,(r.ev_action)::text) > ROW('x'::name, 11854::oid, NULL::"char", NULL::"char", NULL::boolean,
NULL::text,NULL::text))Planning time: 0.201 ms
 
(5 rows)

The code for extracting prefixes of RowCompareExprs like that has existed
for quite some time.  But it doesn't understand about whole-row variables.

>> 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?  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.
        regards, tom lane