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?