On Tue, Jun 16, 2020 at 05:41:31PM +1200, Thomas Munro wrote:
> Thanks! Pushed. I went ahead and made it void in master only.
Thanks.
--
Michael
signature.asc
Description: PGP signature
On Tue, Jun 9, 2020 at 2:32 PM Michael Paquier wrote:
> Looks fine to me. Are you planning to send an extra patch to switch
> BufFileWrite() to void for 14~?
Thanks! Pushed. I went ahead and made it void in master only.
On Mon, Jun 08, 2020 at 05:50:44PM +1200, Thomas Munro wrote:
> On Fri, Jun 5, 2020 at 8:14 PM Michael Paquier wrote:\
>> Anyway, why are we sure that it is fine to not complain even if
>> BufFileWrite() does a partial write? fwrite() is mentioned at the
>> top
>> of the thread, but why is that O
On Tue, Jun 9, 2020 at 2:49 AM Alvaro Herrera wrote:
> I think using our standard "exception" mechanism makes sense. As for
> additional context, I think usefulness of the error messages would be
> improved by showing the file path (because then user knows which
> filesystem/tablespace was full,
On 2020-Jun-08, Thomas Munro wrote:
> Stepping back a bit, one of the problems here is that we tried to
> model the functions on fread(), but we didn't provide the
> companion ferror() and feof() functions, and then we were a bit fuzzy
> on when errno is set, even though the functions don't
> do
On Fri, Jun 5, 2020 at 8:14 PM Michael Paquier wrote:
> On Fri, Jun 05, 2020 at 06:03:59PM +1200, Thomas Munro wrote:
> > I didn't change BufFileWrite() to be void, to be friendly to existing
> > callers outside the tree (if there are any), though I removed all the
> > code that checks the return
On Fri, Jun 05, 2020 at 06:03:59PM +1200, Thomas Munro wrote:
> I didn't change BufFileWrite() to be void, to be friendly to existing
> callers outside the tree (if there are any), though I removed all the
> code that checks the return code. We can make it void later.
Missing one entry in AppendS
On Thu, May 28, 2020 at 7:10 PM Michael Paquier wrote:
> On Wed, May 27, 2020 at 11:59:59AM -0400, Alvaro Herrera wrote:
> > In the discussion that led to 811b6e36a9e2 I did suggest to use "read
> > only M of N" instead, but there wasn't enough discussion on that fine
> > point so we settled on wh
On Wed, May 27, 2020 at 11:59:59AM -0400, Alvaro Herrera wrote:
> In the discussion that led to 811b6e36a9e2 I did suggest to use "read
> only M of N" instead, but there wasn't enough discussion on that fine
> point so we settled on what you now call prevalent without a lot of
> support specificall
On 2020-May-28, Thomas Munro wrote:
> My indecision on the back-patching question has been resolved by your
> crash report and a search on codesearch.debian.org.
Great news!
> BufFileRead() and BufFileWrite() aren't referenced by any of the
> extensions they package, so by that standard it's OK
On Thu, May 28, 2020 at 4:16 AM Alvaro Herrera wrote:
> On 2020-Jan-27, Robert Haas wrote:
> > OK, now that I've waxed eloquent on that topic, let me have a go at
> > your actual questions. Regarding back-patching, I don't mind
> > back-patching error handling patches like this, but I don't feel i
On Wed, May 27, 2020 at 12:16 PM Alvaro Herrera
wrote:
> I do have evidence of postgres crashes because of a problem that could
> be explained by this bug, so I +1 backpatching this to all supported
> branches.
>
> (The problem I saw is a hash-join spilling data to temp tablespace,
> which fills u
On 2020-Jan-27, Robert Haas wrote:
> OK, now that I've waxed eloquent on that topic, let me have a go at
> your actual questions. Regarding back-patching, I don't mind
> back-patching error handling patches like this, but I don't feel it's
> necessary if we have no evidence that data is actually g
On 2020-Jan-29, Michael Paquier wrote:
> On Tue, Jan 28, 2020 at 03:51:54PM -0500, Robert Haas wrote:
> > I quickly reread that thread and I don't see that there's any firm
> > consensus there in favor of "read %d of %zu" over "read only %d of %zu
> > bytes". Now, if most people prefer the former,
Hi Thomas,
On 11/29/19 9:46 PM, Thomas Munro wrote:
Ok. Here is a first attempt at that.
It's been a few CFs since this patch received an update, though there
has been plenty of discussion.
Perhaps it would be best to mark it RwF until you have a chance to
produce an update patch?
Rega
On Mon, Jan 27, 2020 at 7:09 AM Robert Haas wrote:
> Rather than answering your actual question, I'd like to complain about
> this:
>
> if (BufFileRead(file, ptr, BLCKSZ) != BLCKSZ)
> - elog(ERROR, "could not read temporary file: %m");
> + elog(ERROR, "could not read temporary file");
>
> I rec
On Wed, Jan 29, 2020 at 10:01:31AM -0500, Robert Haas wrote:
> Your grep misses one instance of 'read only %d of %d bytes' because
> you grepped for %zu specifically, but that doesn't really change the
> overall picture.
Yes, the one in pg_checksums.c. That could actually be changed with a
cast t
On Wed, Jan 29, 2020 at 1:26 AM Michael Paquier wrote:
> On Tue, Jan 28, 2020 at 03:51:54PM -0500, Robert Haas wrote:
> > I quickly reread that thread and I don't see that there's any firm
> > consensus there in favor of "read %d of %zu" over "read only %d of %zu
> > bytes". Now, if most people pr
On Tue, Jan 28, 2020 at 03:51:54PM -0500, Robert Haas wrote:
> I quickly reread that thread and I don't see that there's any firm
> consensus there in favor of "read %d of %zu" over "read only %d of %zu
> bytes". Now, if most people prefer the former, so be it, but I don't
> think that's clear from
On Mon, Jan 27, 2020 at 9:03 PM Michael Paquier wrote:
> That's actually not the best fit, because this does not take care of
> the pluralization of the second message if you have only 1 byte to
> read ;)
But ... if you have only one byte to read, you cannot have a short read.
> A second point t
On Mon, Jan 27, 2020 at 10:09:30AM -0500, Robert Haas wrote:
> I recognize that your commit message explains this change by saying
> that this code will now never be reached except as a result of a short
> read, but I don't think that will necessarily be clear to future
> readers of those code, or
On Fri, Jan 24, 2020 at 11:12 PM Thomas Munro wrote:
> On Wed, Dec 11, 2019 at 2:07 AM Ibrar Ahmed wrote:
> > You are checking file->dirty twice, first before calling the function and
> > within the function too. Same for the Assert. For example.
>
> True. Thanks for the review. Before I post
On Wed, Dec 11, 2019 at 2:07 AM Ibrar Ahmed wrote:
> You are checking file->dirty twice, first before calling the function and
> within the function too. Same for the Assert. For example.
True. Thanks for the review. Before I post a new version, any
opinions on whether to back-patch, and wheth
You are checking file->dirty twice, first before calling the function and
within the function too. Same for the Assert. For example.
size_t
BufFileRead(BufFile *file, void *ptr, size_t size)
{
size_t nread = 0;
size_t nthistime;
if (file->dirty)
{
BufFi
On Thu, Nov 21, 2019 at 11:31 AM Tom Lane wrote:
> Thomas Munro writes:
> > I think the choices are: (1) switch to ssize_t and return -1, (2) add
> > at least one of BufFileEof(), BufFileError(), (3) have BufFileRead()
> > raise errors with elog(). I lean towards (2), and I doubt we need
> > Buf
Thomas Munro writes:
> As noted by Amit Khandhekar yesterday[1], BufFileLoad() silently eats
> pread()'s error and makes them indistinguishable from EOF.
That's definitely bad.
> I think the choices are: (1) switch to ssize_t and return -1, (2) add
> at least one of BufFileEof(), BufFileError(),
On Thu, Nov 21, 2019 at 10:50 AM Thomas Munro wrote:
> As noted by Amit Khandhekar yesterday[1], BufFileLoad() silently eats
Erm, Khandekar, sorry for the extra h!
Hi,
As noted by Amit Khandhekar yesterday[1], BufFileLoad() silently eats
pread()'s error and makes them indistinguishable from EOF.
Some observations:
1. BufFileRead() returns size_t (not ssize_t), so it's an
fread()-style interface, not a read()-style interface that could use
-1 to signal err
28 matches
Mail list logo
