Re: libpq: PQgetCopyData() and allocation overhead

From: Jeroen Vermeulen <jtvjtv(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jelte Fennema <postgres(at)jeltef(dot)nl>, daniel(at)yesql(dot)se, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: libpq: PQgetCopyData() and allocation overhead
Date: 2023-03-03 18:25:48
Message-ID: CA+zULE7R-Fo0x3K643r1cXEBKBscyL9yct+=DuXJn4jqjQ2uYA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 3 Mar 2023 at 18:14, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Jeroen Vermeulen <jtvjtv(at)gmail(dot)com> writes:
> > On Fri, 3 Mar 2023 at 17:33, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> Let's not do that. Declare it const char *, or maybe better const void
> *.
>
> > Personally I would much prefer "char" over "void" here:
> > * It really is a char buffer, containing text.
>
> Not in binary-mode COPY.
>

True. And in that case zero-termination doesn't matter much either. But
overall libpq's existing choice seems reasonable.

> As for const, I would definitely have preferred that. But if the caller
> > needs a zero-terminated string, forcing them into a memcpy() would kind
> of
> > defeat the purpose.
>
> I'm willing to grant that avoiding malloc-and-free is worth the trouble.
> I'm not willing to allow applications to scribble on libpq buffers to
> avoid memcpy. Even your not-a-patch patch fails to make the case that
> this is essential, because you could have used fwrite() instead of
> printf() (which would be significantly faster yet btw, printf formatting
> ain't cheap).
>

Your house, your rules. For my own use-case "const" is just peachy.

The printf() is just the simplest example that sprang to mind though.
There may be other use-cases out there involving libraries that require
zero-terminated strings, and I figured an ability to set a sentinel could
help those.

> Can do that, sure. I'll also try benchmarking a variant that doesn't take
> > a callback at all, but gives you the buffer pointer in addition to the
> > size/status return. I don't generally like callbacks.
>
> Um ... that would require an assumption that libpq neither changes nor
> moves that buffer before returning to the caller. I don't much like
> that either.
>

Not an assumption about _before returning to the caller_ I guess, because
the function would be on top of that anyway. The concern would be libpq
changing or moving the buffer _before the caller is done with the line._
Which would require some kind of clear rule about what invalidates the
buffer. Yes, that is easier with the callback.

Jeroen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2023-03-03 18:32:53 Re: zstd compression for pg_dump
Previous Message Tom Lane 2023-03-03 18:03:31 Re: pgsql: Harden new test case against force_parallel_mode = regress.