Обсуждение: pgsql: Check for GiST index tuples that don't fit on a page.
Check for GiST index tuples that don't fit on a page. The page splitting code would go into infinite recursion if you try to insert an index tuple that doesn't fit even on an empty page. Per analysis and suggested fix by Andrew Gierth. Fixes bug #11555, reported by Bryan Seitz (analysis happened over IRC). Backpatch to all supported versions. Branch ------ REL9_3_STABLE Details ------- http://git.postgresql.org/pg/commitdiff/ef8ac584e0a7062101dc244566bfe0ca7a13496d Modified Files -------------- src/backend/access/gist/gist.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
On 10/3/14 7:54 AM, Heikki Linnakangas wrote: > Check for GiST index tuples that don't fit on a page. > > The page splitting code would go into infinite recursion if you try to > insert an index tuple that doesn't fit even on an empty page. > > Per analysis and suggested fix by Andrew Gierth. Fixes bug #11555, reported > by Bryan Seitz (analysis happened over IRC). Backpatch to all supported > versions. We don't have portable support for %zu formats until 9.4. The backpatches to 9.3 and earlier need to use %lu.
Peter Eisentraut <peter_e@gmx.net> writes:
> We don't have portable support for %zu formats until 9.4. The
> backpatches to 9.3 and earlier need to use %lu.
Good point --- and something that seems like it'll be an ongoing gotcha.
Have we got any buildfarm critters that could be configured to throw
errors about this?
regards, tom lane
On 10/4/14 12:59 AM, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> We don't have portable support for %zu formats until 9.4. The >> backpatches to 9.3 and earlier need to use %lu. > Good point --- and something that seems like it'll be an ongoing gotcha. > Have we got any buildfarm critters that could be configured to throw > errors about this? If this change had contained a test, it would have caught it. Alternatively, we'd need to catch compiler warnings on the build farm, but if the installation is too old to recognize %zu, who knows what else it'll warn (or not warn) about. Eventually, users of such systems tend point these things out, but that could take a while.
Peter Eisentraut wrote: > On 10/4/14 12:59 AM, Tom Lane wrote: > > Peter Eisentraut <peter_e@gmx.net> writes: > >> We don't have portable support for %zu formats until 9.4. The > >> backpatches to 9.3 and earlier need to use %lu. > > Good point --- and something that seems like it'll be an ongoing gotcha. > > Have we got any buildfarm critters that could be configured to throw > > errors about this? > If this change had contained a test, it would have caught it. > > Alternatively, we'd need to catch compiler warnings on the build farm, > but if the installation is too old to recognize %zu, who knows what else > it'll warn (or not warn) about. Eventually, users of such systems tend > point these things out, but that could take a while. I compiled 9.0 yesterday and there are a lot of warnings with my current compiler; something like -Werror is right out, I think. Maybe we can save warnings separately and have the member turn yellow if there are any, or something like that? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 10/04/2014 05:10 PM, Alvaro Herrera wrote: > Peter Eisentraut wrote: >> On 10/4/14 12:59 AM, Tom Lane wrote: >>> Peter Eisentraut <peter_e@gmx.net> writes: >>>> We don't have portable support for %zu formats until 9.4. The >>>> backpatches to 9.3 and earlier need to use %lu. >>> Good point --- and something that seems like it'll be an ongoing gotcha. >>> Have we got any buildfarm critters that could be configured to throw >>> errors about this? >> If this change had contained a test, it would have caught it. True. I didn't add a test because the error message depends on BLCKSZ and alignment. It also didn't seem like a very likely issue to reoccur in the same form. >> Alternatively, we'd need to catch compiler warnings on the build farm, >> but if the installation is too old to recognize %zu, who knows what else >> it'll warn (or not warn) about. Eventually, users of such systems tend >> point these things out, but that could take a while. > > I compiled 9.0 yesterday and there are a lot of warnings with my current > compiler; something like -Werror is right out, I think. Maybe we can > save warnings separately and have the member turn yellow if there are > any, or something like that? Hmm. For this particular case, it would straightforward to add a step to the buildfarm script to do "grep -r '%z' src". It might turn up false positives, if there's a %z in comments or such, but it shouldn't be much effort to maintain a list of exceptions to filter out the false positives. - Heikki
On Sat, 2014-10-04 at 11:10 -0300, Alvaro Herrera wrote: > I compiled 9.0 yesterday and there are a lot of warnings with my > current > compiler; something like -Werror is right out, I think. Maybe we can > save warnings separately and have the member turn yellow if there are > any, or something like that? This is a very hard problem in depth. You will get totally different warning counts per PostgreSQL version, compiler production, compiler version, probably also different architectures, different operating systems. See https://speakerdeck.com/petere/static-analysis-test-coverage-and-other-12-plus-letter-words slide 6 for example. Add in the general randomness and cruftiness of buildfarm machines, it's a complete crap shoot. Plus you'd need parsers for the output of all the different compilers. See for example https://wiki.jenkins-ci.org/display/JENKINS/Warnings +Plugin .
On Sun, 2014-10-05 at 10:06 +0300, Heikki Linnakangas wrote: > Hmm. For this particular case, it would straightforward to add a step to > the buildfarm script to do "grep -r '%z' src". It might turn up false > positives, if there's a %z in comments or such, but it shouldn't be much > effort to maintain a list of exceptions to filter out the false positives. The problem in this case was that a change was backpatched that doesn't always work in back branches, but works fine in master. The check you describe would only have worked if somehow we'd have added it at the time %z support was added to master, with tremendous foresight. Plus, you don't really want to get into the habit of backpatching more ways the build could fail in stable branches.