This didn't actually work: the COPY succeeds, but the FREEZE
optimization isn't applied. There doesn't seem to be an easy way
to support FREEZE on foreign tables, so let's follow the precedent
established by commit 5c9a5513a3 by raising an error early. This
is arguably a bug fix, but due to the lack of reports, the minimal
discussion on the mailing list, and the potential to break existing
scripts, I am not back-patching it for now.
Author: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com>
Discussion: https://postgr.es/m/CAA5RZ0ujeNgKpE3OrLtR%3DeJGa5LkGMekFzQTwjgw%3DrzaLufQLQ%40mail.gmail.com
We have always documented that a copy EOF marker (\.) must appear
by itself on a line, and that is how psql interprets the rule.
However, the backend's actual COPY FROM logic only insists that
there not be data between the \. and the following newline.
Any data ahead of the \. is parsed as a final line of input.
It's hard to interpret this as anything but an ancient mistake
that we've faithfully carried forward. Continuing to allow it
is not cost-free, since it could mask client-side bugs that
unnecessarily backslash-escape periods (and thereby risk
accidentally creating an EOF marker). So, let's remove that
provision and throw error if the EOF marker isn't alone on its
line, matching what the documentation has said right along.
Adjust the relevant error messages to be clearer, too.
Discussion: https://postgr.es/m/ed659f37-a9dd-42a7-82b9-0da562cc4006@manitou-mail.org
Since backslash is (typically) not special in CSV data, we should
not be treating \. as special either. The server historically did
this to keep CSV and TEXT modes more alike and to support V2 protocol;
but V2 protocol is long dead, and the inconsistency with CSV standards
is annoying. Remove that behavior in CopyReadLineText, and make some
minor consequent code simplifications.
On the client side, we need to fix psql so that it does not check
for \. except when reading data from STDIN (that is, the script
source). We must do that regardless of TEXT/CSV mode or there is
no way to end the COPY short of script EOF. Also, be careful
not to send the \. to the server in that case.
This is a small compatibility break in that other applications
beside psql may need similar adjustment. Also, using an older
version of psql with a v18 server may result in misbehavior
during CSV-mode COPY IN.
Daniel Vérité, reviewed by vignesh C, Robert Haas, and myself
Discussion: https://postgr.es/m/ed659f37-a9dd-42a7-82b9-0da562cc4006@manitou-mail.org
When COPYing into a partitioned table that does now permit the use of
table_multi_insert(), we could error out with
ERROR: could not read block NN in file "base/...": read only 0 of 8192 bytes
because BulkInsertState->next_free was not reset between partitions. This
problem occurred only when not able to use table_multi_insert(), as a
dedicated BulkInsertState for each partition is used in that case.
The bug was introduced in 00d1e02be24, but it was hard to hit at that point,
as commonly bulk relation extension is not used when not using
table_multi_insert(). It became more likely after 82a4edabd27, which expanded
the use of bulk extension.
To fix the bug, reset the bulk relation extension state in BulkInsertState in
ReleaseBulkInsertStatePin(). That was added (in b1ecb9b3fcf) to tackle a very
similar issue. Obviously the name is not quite correct, but there might be
external callers, and bulk insert state needs to be reset in precisely in the
situations that ReleaseBulkInsertStatePin() already needed to be called.
Medium term the better fix likely is to disallow reusing BulkInsertState
across relations.
Add a test that, without the fix, reproduces #18130 in most
configurations. The test also catches the problem fixed in b1ecb9b3fcf when
run with small shared_buffers.
Reported-by: Ivan Kolombet <enderstd@gmail.com>
Analyzed-by: Tom Lane <tgl@sss.pgh.pa.us>
Analyzed-by: Andres Freund <andres@anarazel.de>
Bug: #18130
Discussion: https://postgr.es/m/18130-7a86a7356a75209d%40postgresql.org
Discussion: https://postgr.es/m/257696.1695670946%40sss.pgh.pa.us
Backpatch: 16-
Commit 9f8377f7a2 was a little too eager in fetching default values.
Normally this would not matter, but if the default value is not valid
for the type (e.g. a varchar that's too long) it caused an unnecessary
error.
Complaint and fix from Laurenz Albe
Backpatch to release 16.
Discussion: https://postgr.es/m/75a7b7483aeb331aa017328d606d568fc715b90d.camel@cybertec.at
While on it, newlines are removed from the end of two elog() strings.
The others are simple grammar mistakes. One comment in pg_upgrade
referred incorrectly to sequences since a7e5457.
Author: Justin Pryzby
Discussion: https://postgr.es/m/20221230231257.GI1153@telsasoft.com
Backpatch-through: 11
072132f0 used the attnum offset to access the raw_fields array when
checking that the attribute names of the header and of the relation
match, leading to incorrect results or even crashes if the attribute
numbers of a relation are changed, like on a dropped attribute. This
fixes the logic to use the correct attribute names for the header
matching requirements.
Also, this commit disallows HEADER MATCH in COPY TO as there is no
validation that can be done in this case.
The tests are expanded for HEADER MATCH with COPY FROM and dropped
columns, with cases where a relation has a dropped and re-added column,
as well as a reduced set of columns.
Author: Julien Rouhaud
Reviewed-by: Peter Eisentraut, Michael Paquier
Discussion: https://postgr.es/m/20220607154744.vvmitnqhyxrne5ms@jrouhaud
The idea behind this patch is to make it possible to run individual
test scripts without running the entire core test suite. Making all
the scripts completely independent would involve a massive rewrite,
and would probably be worse for coverage of things like concurrent DDL.
So this patch just does what seems practical with limited changes.
The net effect is that any test script can be run after running
limited earlier dependencies:
* all scripts depend on test_setup
* many scripts depend on create_index
* other dependencies are few in number, and are documented in
the parallel_schedule file.
To accomplish this, I chose a small number of commonly-used tables
and moved their creation and filling into test_setup. Later scripts
are expected not to modify these tables' data contents, for fear of
affecting other scripts' results. Also, our former habit of declaring
all C functions in one place is now gone in favor of declaring them
where they're used, if that's just one script, or in test_setup if
necessary.
There's more that could be done to remove some of the remaining
inter-script dependencies, but significantly more-invasive changes
would be needed, and at least for now it doesn't seem worth it.
Discussion: https://postgr.es/m/1114748.1640383217@sss.pgh.pa.us
"git mv" all the input/*.source and output/*.source files into
the corresponding sql/ and expected/ directories. Then remove
the pg_regress and Makefile infrastructure associated with
dynamic translation.
Discussion: https://postgr.es/m/1655733.1639871614@sss.pgh.pa.us
