Re: pg_prewarm(some more observations in patch)

From: pgsql-hackers-owner(at)postgresql(dot)org [pgsql-hackers-owner(at)postgresql(dot)org] on behalf of Jeff Janes [jeff(dot)janes(at)gmail(dot)com]
>For the implementation:

>1)
>I think that for most users, making them know or care about forks and
> block numbers is too much. It would be nice if there were a
> single-argument form: pg_prewarm(relation) which loaded all of either
> main, or all of all forks, using 'buffer'. This seems like a good
> default. Also, the last two arguments are NULL in all the given
> examples. Do we expect those to be used only for experimental
> purposes by hackers, or are those of general interest?
I agree with you.
2 forms of the function can exist one with only one argument and other
with the arguments as specified in you interface. This can solve the purpose of all kind of users.
In the first form there should be defaults for all other values.

1. For the prewarm type(prefetch,read,buffer), it would have been better if either it is enum or
an case insensitive string. It could have been more convienient from user perspective.

2.
+ if (PG_ARGISNULL(4))
+ last_block = nblocks - 1;
+ else
+ {
+ last_block = PG_GETARG_INT64(4);
+ if (last_block > nblocks)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("ending block number " INT64_FORMAT " exceeds number of blocks in relation " INT64_FORMAT, last_block, nblocks)));
+ }

It can add additional check if last block number is less than first block number, then report meaningful error.
As for this kind of case, it will not load any buffers and returns successfully.

3. + else if (ptype == PREWARM_READ)
+ {
+ /*
+ * In read mode, we actually read the blocks, but not into shared
+ * buffers. This is more portable than prefetch mode (it works
+ * everywhere) and is synchronous.
+ */
+ RelationOpenSmgr(rel);

Is it required to call RelationOpenSmgr(rel) as in the begining already it is done?

With Regards,
Amit Kapila.