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