Date: Thu, 14 Nov 2019 14:00:55 +0100
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Rushabh Lathia <rushabh.lathia@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Andrey Borodin <x4mmm@yandex-team.ru>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	Binguo Bao <djydewang@gmail.com>,
	Paul Ramsey <pramsey@cleverelephant.ca>,
	Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Optimize partial TOAST decompression
Message-ID: <20191114130055.dkrjcveigeviil7n@development>
References: <E11354D1-6980-408D-AC4B-E147EB7CCDED@yandex-team.ru>
 <20190930172951.gdedvexnf4d2wv5e@development>
 <123EF56B-F8EC-4868-B49B-095795095E7A@yandex-team.ru>
 <20191001100805.wnnuj73kuzfwzs56@development>
 <20191001123420.uwzxtqr7qvleyxef@development>
 <20191001131803.j6uin7nho7t6vxzy@development>
 <4411.1569939037@sss.pgh.pa.us>
 <20191001145758.rofbwvguwxwpdxyb@development>
 <2093.1569944075@sss.pgh.pa.us>
 <CAGPqQf2a4+6BKnLC+-b0kEc-02nBfosV=bf8qDRQa+y30N+tNg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="h5nvyo6telceqdoh"
Content-Disposition: inline
In-Reply-To: 
 <CAGPqQf2a4+6BKnLC+-b0kEc-02nBfosV=bf8qDRQa+y30N+tNg@mail.gmail.com>
Precedence: bulk


--h5nvyo6telceqdoh
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline

On Thu, Nov 14, 2019 at 03:27:42PM +0530, Rushabh Lathia wrote:
>Today I noticed strange behaviour, consider the following test:
>
>postgres@126111=#create table foo ( a text );
>CREATE TABLE
>postgres@126111=#insert into foo values ( repeat('PostgreSQL is the
>world''s best database and leading by an Open Source Community.', 8000));
>INSERT 0 1
>
>postgres@126111=#select substring(a from 639921 for 81) from foo;
> substring
>-----------
>
>(1 row)
>

Hmmm. I think the issue is heap_tuple_untoast_attr_slice is using the
wrong way to determine compressed size in the VARATT_IS_EXTERNAL_ONDISK
branch. It does this

     max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
                                             TOAST_COMPRESS_SIZE(attr));

But for the example you've posted TOAST_COMPRESS_SIZE(attr) returns 10,
which is obviously bogus because the TOAST table contains ~75kB of data.

I think it should be doing this instead:

     max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
                                             toast_pointer.va_extsize);

At least that fixes it for me.

I wonder if this actually explains the crashes 540f3168091 was supposed
to fix, but it just masked them instead.

regards

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

--h5nvyo6telceqdoh
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="detoasting.patch"

diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c
index 47a03fa98b..b796406239 100644
--- a/src/backend/access/common/detoast.c
+++ b/src/backend/access/common/detoast.c
@@ -233,7 +233,7 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
 			 * of a given length (after decompression).
 			 */
 			max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
-													TOAST_COMPRESS_SIZE(attr));
+													toast_pointer.va_extsize);
 
 			/*
 			 * Fetch enough compressed slices (compressed marker will get set

--h5nvyo6telceqdoh--