public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
3+ messages / 3 participants
[nested] [flat]
* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)
If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:
- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
backslash (\), and
- the next character is a dot (.), then
CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.
Backpatch to all supported versions.
Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
src/backend/commands/copyfromparse.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
break;
}
else if (!cstate->opts.csv_mode)
-
+ {
/*
* If we are here, it means we found a backslash followed by
* something other than a period. In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
* backslashes are not special, so we want to process the
* character after the backslash just like a normal character,
* so we don't increment in those cases.
+ *
+ * Set 'c' to skip whole character correctly in multi-byte
+ * encodings. If we don't have the whole character in the
+ * buffer yet, we might loop back to process it, after all,
+ * but that's OK because multi-byte characters cannot have any
+ * special meaning.
*/
raw_buf_ptr++;
+ c = c2;
+ }
}
/*
--
2.30.0
--------------CFC80B40DFDD66DF067F288D--
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: micro-optimizing json.c
@ 2023-12-07 23:43 Jeff Davis <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Davis @ 2023-12-07 23:43 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; Joe Conway <[email protected]>; +Cc: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Davin Shearer <[email protected]>; pgsql-hackers
On Thu, 2023-12-07 at 17:12 -0600, Nathan Bossart wrote:
> Here's a patch that removes a couple of strlen() calls that showed up
> prominently in perf for a COPY TO (FORMAT json) on 110M integers. On
> my
> laptop, I see a 20% speedup from ~23.6s to ~18.9s for this test.
Nice improvement. The use of (len = ...) in a conditional is slightly
out of the ordinary, but it makes the conditionals a bit simpler and
you have a comment, so it's fine with me.
I wonder, if there were an efficient cast from numeric to text, then
perhaps you could avoid the strlen() entirely? Maybe there's a way to
use a static buffer to even avoid the palloc() in get_str_from_var()?
Not sure these are worth the effort; just brainstorming.
In any case, +1 to your simple change.
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: micro-optimizing json.c
@ 2023-12-08 00:40 Tom Lane <[email protected]>
parent: Jeff Davis <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Tom Lane @ 2023-12-08 00:40 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Joe Conway <[email protected]>; Andrew Dunstan <[email protected]>; Davin Shearer <[email protected]>; pgsql-hackers
Jeff Davis <[email protected]> writes:
> Nice improvement. The use of (len = ...) in a conditional is slightly
> out of the ordinary, but it makes the conditionals a bit simpler and
> you have a comment, so it's fine with me.
Yeah. It's a little not per project style, but I don't see a nice
way to do it differently, granting that we don't want to put the
strlen() ahead of the !key_scalar test. More straightforward
coding would end up with two else-path calls of escape_json,
which doesn't seem all that much more straightforward.
> I wonder, if there were an efficient cast from numeric to text, then
> perhaps you could avoid the strlen() entirely?
Hmm ... I think that might not be the way to think about it. What
I'm wondering is why we need a test as expensive as IsValidJsonNumber
in the first place, given that we know this is a numeric data type's
output. ISTM we only need to reject "Inf"/"-Inf" and "NaN", which
surely does not require a full parse. Skip over a sign, check for
"I"/"N", and you're done.
... and for that matter, why does quoting of Inf/NaN require
that we apply something as expensive as escape_json? Several other
paths in this switch have no hesitation about assuming that they
can just plaster double quotes around what was emitted. How is
that safe for timestamps but not Inf/NaN?
> In any case, +1 to your simple change.
If we end up not using IsValidJsonNumber then this strlen hackery
would become irrelevant, so maybe that idea should be looked at first.
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-12-08 00:40 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2023-12-07 23:43 Re: micro-optimizing json.c Jeff Davis <[email protected]>
2023-12-08 00:40 ` Re: micro-optimizing json.c Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox