Re: [BUGS] Breakage with VACUUM ANALYSE + partitions

On Fri, Apr 22, 2016 at 1:07 PM, Andres Freund <andres@anarazel.de> wrote:
> The attached patch basically adds the segment size checks to
> _mdfd_getseg(), and doesn't perform extension, even in recovery, if
> EXTENSION_REALLY_RETURN_NULL is passed.
>
> This fixes the issue for me, both in the originally reported variant,
> and in some more rudely setup version (i.e. rm'ing files).

I think this looks broadly reasonable.  Some specific comments:

+                /*
+                 * Normally we will create new segments only if authorized by
+                 * the caller (i.e., we are doing mdextend()).  But when doing
+                 * WAL recovery, create segments anyway; this allows cases
+                 * such as replaying WAL data that has a write into a
+                 * high-numbered segment of a relation that was later deleted.
+                 * We want to go ahead and create the segments so we can
+                 * finish out the replay.

Add something like: "However, if the caller has specified
EXTENSION_REALLY_RETURN_NULL, then extension is not desired even in
recovery; we won't reach this point in that case."

+                    errno = ENOENT;        /* some callers check errno, yuck */

I think a considerably more detailed comment would be warranted.

+                else
+                {
+                    ereport(ERROR,
+                            (errcode_for_file_access(),
+                             errmsg("could not open file \"%s\"
(target block %u): previous segment is only %u blocks",
+                                    _mdfd_segpath(reln, forknum, nextsegno),
+                                    blkno, nblocks)));
+                }

The else and its ensuing level of indentation are unnecessary.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company