On Fri, Jan 6, 2023 at 6:18 PM Peter Eisentraut <peter.eisentr...@enterprisedb.com> wrote: > > On 02.01.23 13:13, Amit Kapila wrote: > > On Wed, Dec 28, 2022 at 4:17 PM Peter Eisentraut > > <peter.eisentr...@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. >
+1. The patch is not getting applied due to a recent commit. > 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. > Yeah, we can do that but not sure if it is worth doing any of those because there are already other places that don't use the exact context. -- With Regards, Amit Kapila.