Re: [BUG] psql: Make \copy from 'text' and 'csv' formats fail on NUL bytes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Joel Jacobson" <joel(at)compiler(dot)org>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [BUG] psql: Make \copy from 'text' and 'csv' formats fail on NUL bytes
Date: 2024-11-10 22:14:38
Message-ID: 163597.1731276878@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Joel Jacobson" <joel(at)compiler(dot)org> writes:
> On Sun, Nov 10, 2024, at 22:37, Tom Lane wrote:
>> That seems like a hack, as it also changes the behavior w.r.t.
>> prompts and EOF-mark detection, neither for the better.

> Hmm, in what way does it change the behavior?
> I ran almost all the tests, including the TAP ones, and none of them failed with
> this fix. Is there some behavior that is currently not covered by the test suite?

Of course. (Our regression tests are very far from covering 100% of
the behavioral space.) In the case at hand, this would break the
expected line-by-line prompting for "\copy ... from '/dev/tty'".

Playing with this just now, I notice that the prompt you get
still claims that \. works to end input, although right now
it does not:

regression=# \copy int8_tbl from '/dev/tty'
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself, or an EOF signal.
>> 1 3
>> 5 6
>> \.
>> ^D
COPY 2

I'm inclined to think that the prompt still describes what should
happen, at least in non-binary mode, although in binary mode we
probably ought to just say (and do) "End with an EOF signal".

So perhaps the if-test to choose the code path could be

if ((isbinary || copystream != pset.cur_cmd_source) && !showprompt)

which would allow dropping the vestigial prompt logic in the first
path, and we would also need to change the test in the second path
that decides if we should check for \. (Likely this should be
refactored a bit to make it more understandable. An intermediate
flag saying whether we intend to check for \. might help.)

Anyway, my point is that we need to think through the desirable
behavior for each possible combination of showprompt, isbinary, and
copystream != pset.cur_cmd_source, because all 8 cases are reachable.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2024-11-10 22:40:43 Re: index prefetching
Previous Message Joel Jacobson 2024-11-10 21:50:59 Re: [BUG] psql: Make \copy from 'text' and 'csv' formats fail on NUL bytes