Re: [PATCH] Fix crash in int8_avg_combine(). - Mailing list pgsql-hackers

From Hadi Moshayedi
Subject Re: [PATCH] Fix crash in int8_avg_combine().
Date
Msg-id CAK=1=WqixaZn8+UfBsCsPNRr2eAp-pTqn4FYcnp7_GrL7rPi=Q@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Fix crash in int8_avg_combine().  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Sat, Nov 25, 2017 at 10:55 PM, Tom Lane wrote: > I believe we already dealt with this: > > Author: Tom Lane > Branch: REL_10_STABLE [619a8c47d] 2017-11-14 17:49:49 -0500 > Branch: REL9_6_STABLE [4a15f87d2] 2017-11-14 17:49:49 -0500 > Branch: REL9_5_STABLE [d4e38489f] 2017-11-14 17:49:49 -0500 > > Prevent int128 from requiring more than MAXALIGN alignment. > > Our initial work with int128 neglected alignment considerations, an > oversight that came back to bite us in bug #14897 from Vincent > Lachenal. > It is unsurprising that int128 might have a 16-byte alignment > requirement; > what's slightly more surprising is that even notoriously lax Intel > chips > sometimes enforce that. > > Raising MAXALIGN seems out of the question: the costs in wasted disk > and > memory space would be significant, and there would also be an on-disk > compatibility break. Nor does it seem very practical to try to allow > some > data structures to have more-than-MAXALIGN alignment requirement, as > we'd > have to push knowledge of that throughout various code that copies data > structures around. > > The only way out of the box is to make type int128 conform to the > system's > alignment assumptions. Fortunately, gcc supports that via its > __attribute__(aligned()) pragma; and since we don't currently support > int128 on non-gcc-workalike compilers, we shouldn't be losing any > platform > support this way. > > Although we could have just done pg_attribute_aligned(MAXIMUM_ALIGNOF) > and > called it a day, I did a little bit of extra work to make the code more > portable than that: it will also support int128 on compilers without > __attribute__(aligned()), if the native alignment of their 128-bit-int > type is no more than that of int64. > > Add a regression test case that exercises the one known instance of the > problem, in parallel aggregation over a bigint column. > > Back-patch of commit 751804998. The code known to be affected only > exists > in 9.6 and later, but we do have some stuff using int128 in 9.5, so > patch > back to 9.5. > > Discussion: https://postgr.es/m/20171110185747.31519.28038@ > wrigleys.postgresql.org > > Does that solution not work for you? > It works for me. My REL_10_STABLE was out of date. A git pull fixed it.

pgsql-hackers by date:

Previous
From: Magnus Hagander
Date:
Subject: Re: [HACKERS] More stats about skipped vacuums
Next
From: Hadi Moshayedi
Date:
Subject: Re: [PATCH] Fix crash in int8_avg_combine().