Re: [BUGS] Breakage with VACUUM ANALYSE + partitions

On Fri, Apr 22, 2016 at 1:07 PM, Andres Freund <andres(at)anarazel(dot)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