Re: mcvstats serialization code is still shy of a load
Tomas Vondra <tomas.vondra@2ndquadrant.com>
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2019-07-02T08:38:29Z
Lists: pgsql-hackers
On Sun, Jun 30, 2019 at 08:30:33PM -0400, Tom Lane wrote: >Tomas Vondra <tomas.vondra@2ndquadrant.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. > Thanks. >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'. > Unfortunately, I was travelling on Sunday and was quite busy on Monday, so I've been unable to push this before the branching :-( I'll push by the end of this week, once I get home. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
Simplify pg_mcv_list (de)serialization
- e74d141d0d97 12.0 landed
- 08aa131c7a72 13.0 landed
-
Sync our Snowball stemmer dictionaries with current upstream
- 7b925e127036 13.0 cited