On 3/1/18 2:19 PM, Peter Geoghegan wrote: > On Thu, Mar 1, 2018 at 11:15 AM, David Steele <da...@pgmasters.net> wrote: >> On 12/15/17 5:31 PM, Peter Geoghegan wrote: >>> Commit d70cf811, from 2014, promoted an Assert() within >>> IndexBuildHeapScan() to a "can't happen" elog() error, in order to >>> detect when a parent tuple cannot be found for some heap-only tuple -- >>> if this happens, then it indicates corruption. I think that we should >>> make it a full ereport(), with an errcode of ERRCODE_DATA_CORRUPTION, >>> to match what Andres just added to code that deals with freezing (he >>> promoted Assert()s to errors, just like the 2014 commit, though he >>> went as far as making them ereport()s to begin with). Attached patch >>> does this. >>> >>> I propose a backpatch to 9.3, partially for the sake of tools like >>> amcheck, where users may only be on the lookout for >>> ERRCODE_DATA_CORRUPTION and ERRCODE_INDEX_CORRUPTED. >> >> This patch applies, passes testing, and appears very straight-forward to >> me. Are there any tests for these conditions currently, or are you only >> doing that in amcheck? > > The enhanced amcheck in the current CF only tests these conditions > indirectly, by pretending to be a CREATE INDEX statement. It matters > to amcheck because these conditions are pretty plausible symptoms of > corruption, and I can imagine someone missing them because they are > not either ERRCODE_DATA_CORRUPTION or ERRCODE_INDEX_CORRUPTED. > > If amcheck didn't exist, then I'd still think that this patch was worthwhile.
Yes, I agree. My thrust was more to discover if there is any testing for these conditions being done in core. It sounds like no, but I don't think it's the responsibility of this patch to add them. I'll mark it Ready for Committer. -- -David da...@pgmasters.net
