Re: Caveats from reloption toast_tuple_target

On Wed, Apr 03, 2019 at 12:13:51PM -0400, Robert Haas wrote:
> On Wed, Apr 3, 2019 at 2:38 AM Michael Paquier <michael@paquier.xyz> wrote:
>> Shouldn't we use the reloption instead of the compiled threshold to
>> determine if a tuple should be toasted or not?  Perhaps I am missing
>> something?  It seems to me that this is a bug that should be
>> back-patched, but it could also be qualified as a behavior change for
>> existing relations.
>
> Could you explain a bit more clearly what you think the bug is?

I mean that toast_tuple_target is broken as-is, because it should be
used on the new tuples of a relation as a threshold to decide if this
tuple should be toasted or not, but we don't actually use the
reloption value for that decision-making: the default threshold
TOAST_TUPLE_THRESHOLD gets used instead all the time!  The code does
not even create a toast table in some cases.

You may want to look at the attached patch if those words make little
sense as code may be easier to explain than words here.  Here is also
a simple example:
CREATE TABLE toto (b text) WITH (toast_tuple_target = 1024);
INSERT INTO toto SELECT string_agg('', md5(random()::text))
  FROM generate_series(1,10); -- 288 bytes
SELECT pg_relation_size(reltoastrelid) = 0 AS is_empty FROM
  pg_class where relname = 'toto';
INSERT INTO toto SELECT string_agg('', md5(random()::text))
  FROM generate_series(1,40); -- 1248 bytes
SELECT pg_relation_size(reltoastrelid) = 0 AS is_empty FROM
  pg_class where relname = 'toto';

On HEAD, the second INSERT does *not* toast the tuple (is_empty is
true).  With the patch attached, the second INSERT toasts the tuple as
I would expect (is_empty is *false*) per the custom threshold
defined.

While on it, I think that the extra argument for
RelationGetToastTupleTarget() is useless as the default value should
be TOAST_TUPLE_THRESHOLD all the time.

Does this make sense?
--
Michael