Re: mcvstats serialization code is still shy of a load

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: mcvstats serialization code is still shy of a load
Date: 2019-07-01 00:30:33
Message-ID: 26200.1561941033@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
> Attached is a slightly improved version of the serialization patch.

I reviewed this patch, and tested it on hppa and ppc. I found one
serious bug: in the deserialization varlena case, you need

- dataptr += MAXALIGN(len);
+ dataptr += MAXALIGN(len + VARHDRSZ);

(approx. line 1100 in mcv.c). Without this, the output data is corrupt,
plus the Assert a few lines further down about dataptr having been
advanced by the correct amount fires. (On one machine I tested on,
that happened during the core regression tests. The other machine
got through regression, but trying to do "select * from pg_stats_ext;"
afterwards exhibited the crash. I didn't investigate closely, but
I suspect the difference has to do with different MAXALIGN values,
4 and 8 respectively.)

The attached patch (a delta atop your v2) corrects that plus some
cosmetic issues.

If we're going to push this, it would be considerably less complicated
to do so before v12 gets branched --- not long after that, there will be
catversion differences to cope with. I'm planning to make the branch
tomorrow (Monday), probably ~1500 UTC. Just sayin'.

regards, tom lane

Attachment Content-Type Size
mcv-serialization-delta.patch text/x-diff 3.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-07-01 01:02:15 Re: Fix typos and inconsistencies for HEAD
Previous Message Tsunakawa, Takayuki 2019-07-01 00:11:58 RE: Commitfest 2019-07, the first of five* for PostgreSQL 13