Take skip header out of a loop in COPY FROM

Started by Surafel Temesgenabout 7 years ago5 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrytests failedCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t41109
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 28, 2026 at 03:53 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t41109_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t41109_1 && git checkout t41109_1

Patchset v1 (message #1) is on t41109_1

Jump to latest
#1Surafel Temesgen
surafel3000@gmail.com

Hello,

Even if skipping header is done only once its checked and skipped in a
loop. If I don’t miss something it can be done out side a loop like
attached patch

regards

Surafel

Attachments:

t41109_1
outing-skip-header-from-loop-v1.patchtext/x-patch; charset=US-ASCII; name=outing-skip-header-from-loop-v1.patchDownload+7-8
#2Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Surafel Temesgen (#1)
Re: Take skip header out of a loop in COPY FROM

On 22/08/2019 11:31, Surafel Temesgen wrote:

Hello,

Even if skipping header is done only once its checked and skipped in a
loop. If I don’t miss something it can be done out side a loop like
attached patch

You may be on to something, but if we move it to CopyFrom(), as in your
patch, then it won't get executed e.g. from the calls in file_fdw.
file_fdw calls BeginCopyFrom(), followed by NextCopyFrom(); it doesn't
use CopyFrom().

- Heikki

#3Adam Lee
ali@pivotal.io
In reply to: Heikki Linnakangas (#2)
Re: Take skip header out of a loop in COPY FROM

On Thu, Aug 22, 2019 at 11:48:31AM +0300, Heikki Linnakangas wrote:

On 22/08/2019 11:31, Surafel Temesgen wrote:

Hello,

Even if skipping header is done only once its checked and skipped in a
loop. If I don’t miss something it can be done out side a loop like
attached patch

You may be on to something, but if we move it to CopyFrom(), as in your
patch, then it won't get executed e.g. from the calls in file_fdw. file_fdw
calls BeginCopyFrom(), followed by NextCopyFrom(); it doesn't use
CopyFrom().

- Heikki

Yes.

My next thought is to call unlikely() here, but we don't have it...
/messages/by-id/CABRT9RC-AUuQL6txxsoOkLxjK1iTpyexpbizRF4Zxny1GXASGg@mail.gmail.com

--
Adam Lee

#4Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Adam Lee (#3)
Re: Take skip header out of a loop in COPY FROM

On 22/08/2019 12:54, Adam Lee wrote:

My next thought is to call unlikely() here, but we don't have it...
/messages/by-id/CABRT9RC-AUuQL6txxsoOkLxjK1iTpyexpbizRF4Zxny1GXASGg@mail.gmail.com

We do, actually, since commit aa3ca5e3dd in v10.

Not sure it's worth the trouble here. Optimizing COPY in general would
be good, even small speedups there are helpful because everyone uses
COPY, but without some evidence I don't believe particular branch is
even measurable.

- Heikki

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Heikki Linnakangas (#4)
Re: Take skip header out of a loop in COPY FROM

Heikki Linnakangas <hlinnaka@iki.fi> writes:

On 22/08/2019 12:54, Adam Lee wrote:

My next thought is to call unlikely() here, but we don't have it...

We do, actually, since commit aa3ca5e3dd in v10.

Not sure it's worth the trouble here. Optimizing COPY in general would
be good, even small speedups there are helpful because everyone uses
COPY, but without some evidence I don't believe particular branch is
even measurable.

I concur that there's no reason to think that this if-test has a
measurable performance cost. We're about to do CopyReadLine which
certainly has way more than one branch's worth of processing in it.

If we want to get involved with sprinkling unlikely() calls into
copy.c, the inner per-character or per-field loops would be the
place to look for wins IMO.

I'm going to mark this CF entry as Returned With Feedback.

regards, tom lane