Date: Thu, 4 Jul 2019 19:46:54 +0200
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: Binguo Bao <djydewang@gmail.com>,
	Paul Ramsey <pramsey@cleverelephant.ca>,
	Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Optimize partial TOAST decompression
Message-ID: <20190704174654.vnvkh7rusv25o6f5@development>
References: 
 <CAL-OGkthU9Gs7TZchf5OWaL-Gsi=hXqufTxKv9qpNG73d5na_g@mail.gmail.com>
 <7B7E4A05-F404-4083-B7F2-267869EE2910@yandex-team.ru>
 <CAL-OGktyTE=Svgv8YTXou4UxjK-zFLtxU6Wy3RndgCesGexwkA@mail.gmail.com>
 <DFBA504A-0E2E-4E9C-8C69-A3C308982A2A@yandex-team.ru>
 <CAL-OGkuBn13N1=jBkiWs4oks_3SxVioiec9Sm9zPDKJFU4szyg@mail.gmail.com>
 <CACowWR1U3guqBzqPL1rHWxYBpU-yUQzr+s07MPT9qBg8LGb+uA@mail.gmail.com>
 <CAL-OGkux7+Bm_J=t5VpH7fJGGSm+PxWJtgs1+WU2g6cmLru==A@mail.gmail.com>
 <BFA726EE-9C16-475D-A739-16DFC34931C1@yandex-team.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <BFA726EE-9C16-475D-A739-16DFC34931C1@yandex-team.ru>
User-Agent: NeoMutt/20180716-1444-295967
Precedence: bulk

On Thu, Jul 04, 2019 at 11:10:24AM +0200, Andrey Borodin wrote:
>
>> 3 июля 2019 г., в 18:06, Binguo Bao <djydewang@gmail.com> написал(а):
>>
>> Paul Ramsey <pramsey@cleverelephant.ca> 于2019年7月2日周二 下午10:46写道：
>> This looks good to me. A little commentary around why
>> pglz_maximum_compressed_size() returns a universally correct answer
>> (there's no way the compressed size can ever be larger than this
>> because...) would be nice for peasants like myself.
>> ...
>>
>> Thanks for your comment. I've updated the patch.
>
>
> Thanks Biguo and Paul! From my POV patch looks ready for committer, so I switched state on CF.
>

I've done a bit of testing and benchmaring on this patch today, and
there's a bug somewhere, making it look like there are corrupted data.

What I'm seeing is this:

CREATE TABLE t (a text);

-- attached is data for one row
COPY t FROM '/tmp/t.data';


SELECT length(substr(a,1000)) from t;
psql: ERROR:  compressed data is corrupted

SELECT length(substr(a,0,1000)) from t;
 length 
--------
    999
(1 row)

SELECT length(substr(a,1000)) from t;
psql: ERROR:  invalid memory alloc request size 2018785106

That's quite bizarre behavior - it does work with a prefix, but not with
suffix. And the exact ERROR changes after the prefix query. (Of course,
on master it works in all cases.)

The backtrace (with the patch applied) looks like this:

#0  toast_decompress_datum (attr=0x12572e0) at tuptoaster.c:2291
#1  toast_decompress_datum (attr=0x12572e0) at tuptoaster.c:2277
#2  0x00000000004c3b08 in heap_tuple_untoast_attr_slice (attr=<optimized out>, sliceoffset=0, slicelength=-1) at tuptoaster.c:315
#3  0x000000000085c1e5 in pg_detoast_datum_slice (datum=<optimized out>, first=<optimized out>, count=<optimized out>) at fmgr.c:1767
#4  0x0000000000833b7a in text_substring (str=133761519127512, start=0, length=<optimized out>, length_not_specified=<optimized out>) at varlena.c:956
...

I've only observed this with a very small number of rows (the  data is
generated randomly with different compressibility etc.), so I'm only
attaching one row that exhibits this issue.

My guess is toast_fetch_datum_slice() gets confused by the headers or
something, or something like that. FWIW the new code added to this
function does not adhere to our code style, and would deserve some
additional explanation of what it's doing/why. Same for the
heap_tuple_untoast_attr_slice, BTW.


regards

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