Re: Add BufFileRead variants with short read and EOF detection

On 02.01.23 13:13, Amit Kapila wrote:
> On Wed, Dec 28, 2022 at 4:17 PM Peter Eisentraut
> <peter.eisentraut@enterprisedb.com> wrote:
>>
>> Most callers of BufFileRead() want to check whether they read the full
>> specified length.  Checking this at every call site is very tedious.
>> This patch provides additional variants BufFileReadExact() and
>> BufFileReadMaybeEOF() that include the length checks.
>>
>> I considered changing BufFileRead() itself, but this function is also
>> used in extensions, and so changing the behavior like this would create
>> a lot of problems there.  The new names are analogous to the existing
>> LogicalTapeReadExact().
>>
> 
> +1 for the new APIs. I have noticed that some of the existing places
> use %m and the file path in messages which are not used in the new
> common function.

The existing uses of %m are wrong.  This was already fixed once in 
7897e3bb902c557412645b82120f4d95f7474906, but the affected areas of code 
were apparently developed at around the same time and didn't get the 
fix.  So I have attached a separate patch to fix this first, which could 
be backpatched.

The original patch is then rebased on top of that.  I have adjusted the 
error message to include the file set name if available.

What this doesn't keep is the purpose of the temp file in some cases, 
like "hash-join temporary file".  We could maybe make this an additional 
argument or an error context, but it seems cumbersome in any case. 
Thoughts?