Re: mcvstats serialization code is still shy of a load
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2019-07-01T00:30:33Z
Lists: pgsql-hackers
Attachments
- mcv-serialization-delta.patch (text/x-diff) patch
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. 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
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