On Tue, Feb 25, 2020 at 1:09 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
>
> On 2020-02-24 12:21, Kuntal Ghosh wrote:
> > I've reproduced the issue on head. And, the patch seems to solve the
> > problem. The patch looks good to me. But, I've a small doubt regarding
> > the changes in test_decoding regression file.
> >
> > diff --git a/contrib/test_decoding/sql/toast.sql
> > b/contrib/test_decoding/sql/toast.sql
> > ..
> > -INSERT INTO toasted_several(toasted_key) VALUES(repeat('9876543210', 10000));
> > -SELECT pg_column_size(toasted_key) > 2^16 FROM toasted_several;
> > +INSERT INTO toasted_several(toasted_key) VALUES(repeat('9876543210', 269));
> > +SELECT pg_column_size(toasted_key) > 2^11 FROM toasted_several;
> >
> > This actually tests whether we can decode "old" tuples bigger than the
> > max heap tuple size correctly which is around 8KB. But, the above
> > changes will make the tuple size around 3KB. So, it'll not be able to
> > test that particular scenario.Thoughts?
>
> OK, this is interesting. The details of this are somewhat unfamiliar to
> me, but it appears that due to TOAST_INDEX_HACK in indextuple.c, an
> index tuple cannot be larger than 8191 bytes when untoasted (but not
> uncompressed).
>
> What the test case above is testing is a situation where the heap tuple
> is stored toasted uncompressed (storage external) but the index tuple is
> not (probably compressed inline). This is exactly the situation that I
> was contending should not be possible, because it cannot be dumped or
> restored.
>
Yeah. If we only commit this patch to fix the issue, we're going to
put some restriction for the above situation, i.e., the index for an
external attribute has to be stored as an external (i.e. uncompressed)
value. So, a lot of existing workload might start failing after an
upgrade. I think there should be an option to store the index of an
external attribute as a compressed inline value.
> An alternative would be that we make this situation fully supported.
> Then we'd probably need at least ALTER INDEX ... ALTER COLUMN ... SET
> STORAGE, and some pg_dump support.
>
> Thoughts?
Yes. We need the support for this syntax along with the bug fix patch.
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com