agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Nathan Bossart <nathandbossart@gmail.com>
To: Greg Burd <greg@burd.me>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>
Cc: Manni Wood <manni.wood@enterprisedb.com>
Cc: KAZAR Ayoub <ma_kazar@esi.dz>
Cc: Neil Conway <neil.conway@gmail.com>
Cc: Andrew Dunstan <andrew@dunslane.net>
Cc: Shinya Kato <shinya11.kato@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Speed up COPY FROM text/CSV parsing using SIMD
Date: Mon, 31 Aug 2026 09:24:47 -0500
Message-ID: <apWOr1kgVSqweBnU@nathan> (raw)
In-Reply-To: <abRKUiInG6Gu728Q@nathan>
References: <abHTvkeIK37hj9oS@nathan>
<CAN55FZ1sn-2tVX_n9C5UNBCfDPjSDOCT4zkMeDsB7MaZ9SUBTw@mail.gmail.com>
<abL5ylYjmopRRmvy@nathan>
<CAN55FZ0oYuVHZ=TdJofj0WA3mG77XumVsGJgG8gnav9kexr+wg@mail.gmail.com>
<CAN55FZ0ocS6cBHEWqHv2s-dK91U6OdVLBqj7VexTehtBtioDbA@mail.gmail.com>
<abQZmZ0KZesCxfR0@nathan>
<43de48dc-701b-4735-881b-50bca6870f39@app.fastmail.com>
<CAN55FZ1b1Jstrr-K7xcXY9z1ObDRxi48dAhwdHpTZn1C=S-jHA@mail.gmail.com>
<b26a1cf8-335d-4054-90ad-d94457c45947@app.fastmail.com>
<abRKUiInG6Gu728Q@nathan>
Claude found this bug:
CREATE TABLE t (a TEXT);
COPY t FROM PROGRAM $$printf 'abcdefghijklmnopqrst\n\\.\nxxxxxxxxxxxx\200'$$;
fails with
ERROR: invalid byte sequence for encoding "UTF8": 0x80
CONTEXT: COPY t, line 2
even though COPY should ignore everything after the \.
The best fix I could find involves teaching CopyLoadInputBuf() to avoid
erroring in the read-ahead path. AFAICT that doesn't meaningfully change
the performance characteristics. Patch attached.
--
nathan
From d9a9466a2f986c24d996110ffd691c5965509e8b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 31 Aug 2026 09:08:23 -0500
Subject: [PATCH v1 1/1] Make COPY FROM's SIMD read-ahead speculative.
Commit e0a3a3fd53 added a SIMD scan for COPY FROM (FORMAT
{text,csv}) that refills the input buffer whenever fewer than
sizeof(Vector8) bytes remain, rather than when it has actually run
out. Presently, that read-ahead calls CopyLoadInputBuf() where
the scalar loop would not, and CopyLoadInputBuf() is where a
deferred encoding error is reported. So a file whose data ends
with \. followed by bytes that are invalid in the encoding, a case
CopyConvertBuf() goes out of its way to tolerate, now fails.
Whether it fails depends on how much never-examined padding sits
between the marker and the invalid byte: with 16-byte vectors, 12
bytes or fewer and the COPY errors out, 13 or more and it
succeeds.
To fix, teach CopyLoadInputBuf() to tell a speculative load, made
on the chance that the caller will want the data, from one the
caller needs in order to make progress. A speculative load leaves
a pending encoding error pending, so long as the caller still has
bytes to chew on. The SIMD path then hands those bytes to the
scalar code, which asks only for what it needs and so never reads
past the marker. Input that is really read is unaffected, except
that its errors are once again reported against the line holding
the bad byte, as they were before e0a3a3fd53.
The obvious fix, declining to read ahead at all, is worse than it
looks. The SIMD helper runs once per line, so refusing to refill
hands the entire remainder of any line that straddles a buffer
boundary to the scalar loop, and COPY of megabyte-wide lines
slowed by about 80% in my testing. Consuming the sub-vector tail
inside the helper instead cost the compiler its unrolling of the
vector loop, which was worse again. Leaving that loop untouched,
as this patch does, measures within noise of unpatched on every
line width I tried.
Oversight in commit e0a3a3fd53.
Discussion: https://postgr.es/m/CAOzEurSW8cNr6TPKsjrstnPfhf4QyQqB4tnPXGGe8N4e_v7Jig%40mail.gmail.com
Backpatch-through: 19
---
src/backend/commands/copyfromparse.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 37750cca13a..dc02838bc38 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -167,7 +167,7 @@ static int CopyGetData(CopyFromState cstate, void *databuf,
int minread, int maxread);
static inline bool CopyGetInt32(CopyFromState cstate, int32 *val);
static inline bool CopyGetInt16(CopyFromState cstate, int16 *val);
-static void CopyLoadInputBuf(CopyFromState cstate);
+static void CopyLoadInputBuf(CopyFromState cstate, bool speculative);
static int CopyReadBinaryData(CopyFromState cstate, char *dest, int nbytes);
void
@@ -651,9 +651,15 @@ CopyLoadRawBuf(CopyFromState cstate)
*
* If INPUT_BUF_BYTES(cstate) > 0, the unprocessed bytes are moved to the start
* of the buffer and then we load more data after that.
+ *
+ * A speculative load is one made on the chance that the caller will want the
+ * data, not because it needs it yet. Such a load leaves a pending encoding
+ * error pending, so long as the caller still has something to chew on, since
+ * the input may never be read that far. NB: a speculative caller must cope
+ * with getting no additional data.
*/
static void
-CopyLoadInputBuf(CopyFromState cstate)
+CopyLoadInputBuf(CopyFromState cstate, bool speculative)
{
int nbytes = INPUT_BUF_BYTES(cstate);
@@ -684,7 +690,11 @@ CopyLoadInputBuf(CopyFromState cstate)
* conversion error.
*/
if (cstate->input_reached_error)
+ {
+ if (speculative && INPUT_BUF_BYTES(cstate) > 0)
+ return;
CopyConversionError(cstate);
+ }
/* no more input, and everything has been converted */
if (cstate->input_reached_eof)
@@ -1381,7 +1391,7 @@ CopyReadLineTextSIMDHelper(CopyFromState cstate, bool is_csv,
{
REFILL_LINEBUF;
- CopyLoadInputBuf(cstate);
+ CopyLoadInputBuf(cstate, true);
/* update our local variables */
*hit_eof_p = cstate->input_reached_eof;
input_buf_ptr = cstate->input_buf_index;
@@ -1566,7 +1576,7 @@ CopyReadLineText(CopyFromState cstate, bool is_csv)
{
REFILL_LINEBUF;
- CopyLoadInputBuf(cstate);
+ CopyLoadInputBuf(cstate, false);
/* update our local variables */
hit_eof = cstate->input_reached_eof;
input_buf_ptr = cstate->input_buf_index;
--
2.55.0
Attachments:
[text/plain] v1-0001-Make-COPY-FROM-s-SIMD-read-ahead-speculative.patch (4.5K, ../apWOr1kgVSqweBnU@nathan/2-v1-0001-Make-COPY-FROM-s-SIMD-read-ahead-speculative.patch)
download | inline diff:
From d9a9466a2f986c24d996110ffd691c5965509e8b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 31 Aug 2026 09:08:23 -0500
Subject: [PATCH v1 1/1] Make COPY FROM's SIMD read-ahead speculative.
Commit e0a3a3fd53 added a SIMD scan for COPY FROM (FORMAT
{text,csv}) that refills the input buffer whenever fewer than
sizeof(Vector8) bytes remain, rather than when it has actually run
out. Presently, that read-ahead calls CopyLoadInputBuf() where
the scalar loop would not, and CopyLoadInputBuf() is where a
deferred encoding error is reported. So a file whose data ends
with \. followed by bytes that are invalid in the encoding, a case
CopyConvertBuf() goes out of its way to tolerate, now fails.
Whether it fails depends on how much never-examined padding sits
between the marker and the invalid byte: with 16-byte vectors, 12
bytes or fewer and the COPY errors out, 13 or more and it
succeeds.
To fix, teach CopyLoadInputBuf() to tell a speculative load, made
on the chance that the caller will want the data, from one the
caller needs in order to make progress. A speculative load leaves
a pending encoding error pending, so long as the caller still has
bytes to chew on. The SIMD path then hands those bytes to the
scalar code, which asks only for what it needs and so never reads
past the marker. Input that is really read is unaffected, except
that its errors are once again reported against the line holding
the bad byte, as they were before e0a3a3fd53.
The obvious fix, declining to read ahead at all, is worse than it
looks. The SIMD helper runs once per line, so refusing to refill
hands the entire remainder of any line that straddles a buffer
boundary to the scalar loop, and COPY of megabyte-wide lines
slowed by about 80% in my testing. Consuming the sub-vector tail
inside the helper instead cost the compiler its unrolling of the
vector loop, which was worse again. Leaving that loop untouched,
as this patch does, measures within noise of unpatched on every
line width I tried.
Oversight in commit e0a3a3fd53.
Discussion: https://postgr.es/m/CAOzEurSW8cNr6TPKsjrstnPfhf4QyQqB4tnPXGGe8N4e_v7Jig%40mail.gmail.com
Backpatch-through: 19
---
src/backend/commands/copyfromparse.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 37750cca13a..dc02838bc38 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -167,7 +167,7 @@ static int CopyGetData(CopyFromState cstate, void *databuf,
int minread, int maxread);
static inline bool CopyGetInt32(CopyFromState cstate, int32 *val);
static inline bool CopyGetInt16(CopyFromState cstate, int16 *val);
-static void CopyLoadInputBuf(CopyFromState cstate);
+static void CopyLoadInputBuf(CopyFromState cstate, bool speculative);
static int CopyReadBinaryData(CopyFromState cstate, char *dest, int nbytes);
void
@@ -651,9 +651,15 @@ CopyLoadRawBuf(CopyFromState cstate)
*
* If INPUT_BUF_BYTES(cstate) > 0, the unprocessed bytes are moved to the start
* of the buffer and then we load more data after that.
+ *
+ * A speculative load is one made on the chance that the caller will want the
+ * data, not because it needs it yet. Such a load leaves a pending encoding
+ * error pending, so long as the caller still has something to chew on, since
+ * the input may never be read that far. NB: a speculative caller must cope
+ * with getting no additional data.
*/
static void
-CopyLoadInputBuf(CopyFromState cstate)
+CopyLoadInputBuf(CopyFromState cstate, bool speculative)
{
int nbytes = INPUT_BUF_BYTES(cstate);
@@ -684,7 +690,11 @@ CopyLoadInputBuf(CopyFromState cstate)
* conversion error.
*/
if (cstate->input_reached_error)
+ {
+ if (speculative && INPUT_BUF_BYTES(cstate) > 0)
+ return;
CopyConversionError(cstate);
+ }
/* no more input, and everything has been converted */
if (cstate->input_reached_eof)
@@ -1381,7 +1391,7 @@ CopyReadLineTextSIMDHelper(CopyFromState cstate, bool is_csv,
{
REFILL_LINEBUF;
- CopyLoadInputBuf(cstate);
+ CopyLoadInputBuf(cstate, true);
/* update our local variables */
*hit_eof_p = cstate->input_reached_eof;
input_buf_ptr = cstate->input_buf_index;
@@ -1566,7 +1576,7 @@ CopyReadLineText(CopyFromState cstate, bool is_csv)
{
REFILL_LINEBUF;
- CopyLoadInputBuf(cstate);
+ CopyLoadInputBuf(cstate, false);
/* update our local variables */
hit_eof = cstate->input_reached_eof;
input_buf_ptr = cstate->input_buf_index;
--
2.55.0
view thread (178+ messages) latest in thread
Message-ID: <apWOr1kgVSqweBnU@nathan>
Permalink: ../apWOr1kgVSqweBnU@nathan/
Also on: postgresql.org/message-id/apWOr1kgVSqweBnU@nathan
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: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, greg@burd.me, byavuz81@gmail.com, manni.wood@enterprisedb.com, ma_kazar@esi.dz, neil.conway@gmail.com, andrew@dunslane.net, shinya11.kato@gmail.com
Subject: Re: Speed up COPY FROM text/CSV parsing using SIMD
In-Reply-To: <apWOr1kgVSqweBnU@nathan>
* 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