Re: Fix brin_form_tuple to properly detoast data - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: Fix brin_form_tuple to properly detoast data
Date
Msg-id 69ede68a-01ea-7c2c-4985-39c7efb6e8ce@2ndquadrant.com
Whole thread Raw
In response to Fix brin_form_tuple to properly detoast data  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
Responses Re: Fix brin_form_tuple to properly detoast data  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers
On 11/4/20 2:05 AM, Tomas Vondra wrote:
> Hi,
> 
> As pointed out in [1], BRIN is not properly handling toasted data, which
> may easily lead to index tuples referencing TOAST-ed values. Which is
> clearly wrong - it's trivial to trigger failues after a DELETE.
> 
> Attached is a patch that aims to fix this - AFAIK the brin_form_tuple
> was simply missing the TOAST_INDEX_HACK stuff from index_form_tuple,
> which ensures the data is detoasted and (possibly) re-compressed. The
> code is mostly the same, with some BRIN-specific tweaks (looking at
> oi_typecache instead of the index descriptor, etc.).
> 
> I also attach a simple SQL script that I used to trigger the issue. This
> needs to be turned into a regression test, I'll work on that tomorrow.
> 

OK, so here's an improved version of the fix - aside from the code (same 
as in v1), there are two patches with regression tests. Ultimately those 
should be merged with the fix, but this way it's possible to apply the 
regression tests to trigger the issue.

The first test is fairly trivial - it simply builds index on toasted 
data and then shows how an insert and select fail. There's a caveat, 
that this requires a DELETE + VACUUM, and the VACUUM actually has to 
cleanup the rows. So there must be no concurrent transactions that might 
need the rows, which is unlikely in regression tests. So this requires 
waiting for all running transactions to finish - I did that by building 
an index concurrently. It's a bit strange, but it's better than any 
other solution I could think of (timeout or some custom wait for xacts).

The second test is a bit redundant - it merely checks that both CREATE 
INDEX and INSERT INTO fail the same way when the index tuple gets too 
large. Before the fix there were some inconsistencies - the CREATE INDEX 
succeeded because it used TOASTed data. So ultimately this tests the 
same thing, but from a different perspective.


regards


-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: Use of "long" in incremental sort code
Next
From: Melanie Plageman
Date:
Subject: Re: Parallel Full Hash Join