public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Andres Freund <[email protected]>
Cc: Sutou Kouhei <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
Date: Wed, 7 Feb 2024 13:33:18 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On Tue, Feb 06, 2024 at 03:33:36PM -0800, Andres Freund wrote:
> Well, you can't just do that, because there's only one caller, namely
> CopyToTextOneRow(). What I am trying to suggest is something like the
> attached, just a quick hacky POC. Namely to split out CSV support from
> CopyToTextOneRow() by introducing CopyToCSVOneRow(), and to avoid code
> duplication by moving the code into a new CopyToTextLikeOneRow().

Ah, OK.  Got it now.

> I named it CopyToTextLike* here, because it seems confusing that some Text*
> are used for both CSV and text and others are actually just for text. But if
> were to go for that, we should go further.

This can always be argued later.

> To test the performnce effects I chose to remove the pointless encoding
> "check" we're discussing in the other thread, as it makes it harder to see the
> time differences due to the per-attribute code.  I did three runs of pgbench
> -t of [1] and chose the fastest result for each.
> 
> With turbo mode and power saving disabled:
>                           Avg Time
> HEAD                       995.349
> Remove Encoding Check      870.793
> v13-0001                   869.678
> Remove out callback        839.508

Hmm.  That explains why I was not seeing any differences with this
callback then.  It seems to me that the order of actions to take is
clear, like:
- Revert 2889fd23be56 to keep a clean state of the tree, now done with
1aa8324b81fa.
- Dive into the strlen() issue, as it really looks like this can
create more simplifications for the patch discussed on this thread
with COPY TO.
- Revisit what we have here, looking at more profiles to see how HEAD
an v13 compare.  It looks like we are on a good path, but let's tackle
things one step at a time.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (13+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox