RE: pg_dump not dumping the run_as_owner setting from version 16?
> > I as far as I can tell, pg_dump does not dup the ‘run_as_owner` setting for > > a subscription. > > > > Should it? Should I submit a patch? It seems pretty trivial to fix if > > anyone else is working on it. > > Yes, it certainly should. That is an omission in 482675987b. > Go ahead and write a fix! Please find attached a patch for pg_dump to honour the setting of `run_as_owner`; I believe that effective pre-16 behavious was to run as owner, so I have set the flag to ‘t’ for pre-16 versions. Please let me know if you would prefer the opposite. > > Further to this: it seems that `Alter Subscription X > > Set(Run_As_Owner=True);` > > has no influence on the `subrunasowner` column of pg_subscriptions. > > This seems to have been fixed in f062cddafe. Yes, I can confirm that in the current head `pg_subscriptions` reflects the setting correctly.
Re: SQL:2011 application time
V16 patch doc/src/sgml/html/sql-createtable.html doc SET NULL description:
`
SET NULL [ ( column_name [, ... ] ) ]
Set all of the referencing columns, or a specified subset of the
referencing columns, to null. A subset of columns can only be
specified for ON DELETE actions.
In a temporal foreign key, the change will use FOR PORTION OF
semantics to constrain the effect to the bounds of the referenced row.
`
I think it means, if the foreign key has PERIOD column[s], then the
PERIOD column[s] will not be set to NULL in {ON DELETE|ON UPDATE}. We
can also use FOR PORTION OF semantics to constrain the effect to the
bounds of the referenced row.
see below demo:
BEGIN;
drop table if exists temporal_rng CASCADE;
drop table if exists temporal_fk_rng2rng CASCADE;
CREATE unlogged TABLE temporal_rng (id int4range,valid_at tsrange);
ALTER TABLE temporal_rng ADD CONSTRAINT temporal_rng_pk PRIMARY KEY
(id, valid_at WITHOUT OVERLAPS);
CREATE unlogged TABLE temporal_fk_rng2rng (id int4range,valid_at
tsrange,parent_id int4range,
CONSTRAINT temporal_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD valid_at)
REFERENCES temporal_rng (id, PERIOD valid_at) on update set null ON
DELETE SET NULL);
INSERT INTO temporal_rng VALUES ('[11,11]', tsrange('2018-01-01',
'2021-01-01'));
INSERT INTO temporal_fk_rng2rng VALUES ('[7,7]', tsrange('2018-01-01',
'2021-01-01'), '[11,11]');
DELETE FROM temporal_rng WHERE id = '[11,11]';
table temporal_fk_rng2rng;
commit;
-
also
"REFERENCES temporal_rng (id, PERIOD valid_at) ON UPDATE SET NULL ON
DELETE SET NULL)"
is the same as
"REFERENCES temporal_rng (id, PERIOD valid_at) ON UPDATE SET NULL ON
DELETE SET NULL (parent_id)"
in the current implementation.
we might need to change the pg_constraint column "confdelsetcols" description.
---
the above also applies to SET DEFAULT.
--
can you add the following for the sake of code coverage. I think
src/test/regress/sql/without_overlaps.sql can be simplified.
--- common template for test foreign key constraint.
CREATE OR REPLACE PROCEDURE overlap_template()
LANGUAGE SQL
AS $$
DROP TABLE IF EXISTS temporal_rng CASCADE;
DROP TABLE IF EXISTS temporal_fk_rng2rng CASCADE;
CREATE UNLOGGED TABLE temporal_rng (id int4range,valid_at tsrange);
ALTER TABLE temporal_rng
ADD CONSTRAINT temporal_rng_pk PRIMARY KEY (id, valid_at WITHOUT OVERLAPS);
CREATE UNLOGGED TABLE temporal_fk_rng2rng (
id int4range,
valid_at tsrange,
parent_id int4range,
CONSTRAINT temporal_fk_rng2rng_fk
FOREIGN KEY (parent_id, PERIOD valid_at)
REFERENCES temporal_rng (id, PERIOD valid_at)
ON UPDATE no action ON DELETE no action
DEFERRABLE
);
$$;
call overlap_template();
--- on update/delete restrict
-- coverage for TRI_FKey_restrict_upd,TRI_FKey_restrict_del.
BEGIN;
ALTER TABLE temporal_fk_rng2rng
DROP CONSTRAINT temporal_fk_rng2rng_fk,
ADD CONSTRAINT temporal_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD valid_at)
REFERENCES temporal_rng(id,PERIOD valid_at) ON UPDATE RESTRICT ON
DELETE RESTRICT;
INSERT INTO temporal_rng VALUES ('[11,11]', tsrange('2018-01-01',
'2021-01-01'));
INSERT INTO temporal_fk_rng2rng VALUES ('[7,7]', tsrange('2018-01-01',
'2020-01-01'), '[11,11]');
savepoint s;
UPDATE temporal_rng FOR PORTION OF valid_at FROM '2018-01-01' TO '2018-01-03'
SET id = '[9,9]' WHERE id = '[11,11]';
ROLLBACK to s;
delete from temporal_rng FOR PORTION OF valid_at FROM '2018-01-01' TO
'2020-01-01';
ROLLBACK to s;
--this one should not have error.
delete from temporal_rng FOR PORTION OF valid_at FROM '2020-01-01' TO
'2021-01-01';
table temporal_rng;
ROLLBACK;
-
--- on delete set column list coverage for function tri_set. branch
{if (riinfo->ndelsetcols != 0)}
BEGIN;
ALTER TABLE temporal_fk_rng2rng
DROP CONSTRAINT temporal_fk_rng2rng_fk,
ADD CONSTRAINT temporal_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD valid_at)
REFERENCES temporal_rng(id,PERIOD valid_at) ON DELETE set default(parent_id);
ALTER TABLE temporal_fk_rng2rng ALTER COLUMN parent_id SET DEFAULT '[2,2]';
ALTER TABLE temporal_fk_rng2rng ALTER COLUMN valid_at SET DEFAULT tsrange'(,)';
INSERT INTO temporal_rng VALUES ('[11,11]', tsrange('2018-01-01',
'2021-01-01'));
INSERT INTO temporal_fk_rng2rng VALUES ('[7,7]', tsrange('2018-01-01',
'2020-01-01'), '[11,11]');
insert into temporal_rng values('[2,2]','(,)');
savepoint s;
delete from temporal_rng FOR PORTION OF valid_at FROM '2018-01-01' TO
'2019-01-01' where id = '[11,11]';
-- delete from temporal_rng where id = '[11,11]';
table temporal_fk_rng2rng;
rollback;
Re: Supporting MERGE on updatable views
hi. Excellent work! regress test passed! The code looks so intuitive! doc/src/sgml/ref/create_view.sgml. Do we need to addri_RowIdAttNo,&isNull); . oldtupdata.t_data = DatumGetHeapTupleHeader(datum); oldtupdata.t_len = HeapTupleHeaderGetDatumLength(oldtupdata.t_data); ` In ExecGetJunkAttribute(slot,resultRelInfo->ri_RowIdAttNo,&isNull); does resultRelInfo->ri_RowIdAttNo must be 1 to make sure DatumGetHeapTupleHeader(datum) works? (I am not familiar with this part.)
Re: unnest multirange, returned order
On Fri, 2023-10-27 at 16:08 -0700, Jeff Davis wrote: > On Fri, 2023-10-27 at 08:48 +0200, Laurenz Albe wrote: > > On Fri, 2023-10-13 at 15:33 -0400, Daniel Fredouille wrote: > > > sorry it took me some time to reply. Yes, the patch is perfect if > > > this is indeed the behavior. > > > > I'm sending a reply to the hackers list so that I can add the patch > > to the commitfest. > > > > Tiny as the patch is, I don't want it to fall between the cracks. > > Committed with adjusted wording. Thank you! Thanks! Yours, Laurenz Albe
RE: pg_dump not dumping the run_as_owner setting from version 16?
...patch actually attached this time... > > I as far as I can tell, pg_dump does not dup the ‘run_as_owner` setting for > > a subscription. > > > > Should it? Should I submit a patch? It seems pretty trivial to fix if > > anyone else is working on it. > > Yes, it certainly should. That is an omission in 482675987b. > Go ahead and write a fix! Please find attached a patch for pg_dump to honour the setting of `run_as_owner`; I believe that effective pre-16 behavious was to run as owner, so I have set the flag to ‘t’ for pre-16 versions. Please let me know if you would prefer the opposite. > > Further to this: it seems that `Alter Subscription X > > Set(Run_As_Owner=True);` > > has no influence on the `subrunasowner` column of pg_subscriptions. > > This seems to have been fixed in f062cddafe. Yes, I can confirm that in the current head `pg_subscriptions` reflects the setting correctly. pg_dump-run_as_owner.patch Description: Binary data
Re: [PoC] Improve dead tuple storage for lazy vacuum
I wrote: > Seems fine at a glance, thanks. I will build on this to implement > variable-length values. I have already finished one prerequisite which is: > public APIs passing pointers to values. Since my publishing schedule has not kept up, I'm just going to share something similar to what I mentioned earlier, just to get things moving again. 0001-0009 are from earlier versions, except for 0007 which makes a bunch of superficial naming updates, similar to those done in a recent other version. Somewhere along the way I fixed long-standing git whitespace warnings, but I don't remember if that's new here. In any case, let's try to preserve that. 0010 is some minor refactoring to reduce duplication 0011-0014 add public functions that give the caller more control over the input and responsibility for locking. They are not named well, but I plan these to be temporary: They are currently used for the tidstore only, since that has much simpler tests than the standard radix tree tests. One thing to note: since the tidstore has always done it's own locking within a larger structure, these patches don't bother to do locking at the radix tree level. Locking twice seems...not great. These patches are the main prerequisite for variable-length values. Once that is working well, we can switch the standard tests to the new APIs. Next steps include (some of these were briefly discussed off-list with Sawada-san): - template parameter for varlen values - some callers to pass length in bytes - block entries to have num_elems for # of bitmap words - a way for updates to re-alloc values when needed - aset allocation for values when appropriate v43-ART.tar.gz Description: application/gzip
Re: Version 14/15 documentation Section "Alter Default Privileges"
Hi, On Fri, Oct 27, 2023 at 05:49:42PM +0200, Laurenz Albe wrote: > On Fri, 2023-10-27 at 11:34 +0200, Michael Banck wrote: > > On Fri, Oct 27, 2023 at 09:03:04AM +0200, Laurenz Albe wrote: > > > On Fri, 2022-11-04 at 10:49 +0100, Laurenz Albe wrote: > > > > On Thu, 2022-11-03 at 11:32 +0100, Laurenz Albe wrote: > > > > > On Wed, 2022-11-02 at 19:29 +, David Burns wrote: > > > > > > Some additional clarity in the versions 14/15 documentation > > > > > > would be helpful specifically surrounding the "target_role" > > > > > > clause for the ALTER DEFAULT PRIVILEGES command. To the > > > > > > uninitiated, the current description seems vague. Maybe > > > > > > something like the following would help: > > > > > > > > After some more thinking, I came up with the attached patch. > > > > > I think something like this is highly useful because I have also seen > > people very confused why default privileges are not applied. > > > > However, maybe it could be made even clearer if also the main > > description is amended, like > > > > "You can change default privileges only for objects that will be created > > by yourself or by roles that you are a member of (via target_role)." > > > > or something. > > True. I have done that in the attached patch. > In this patch, it is mentioned *twice* that ALTER DEFAULT PRIVILEGES > only affects objects created by the current user. I thought that > would not harm, but if it is too redundant, I can remove the second > mention. I think it is fine, and I have marked the patch as ready-for-committer. I think it should be applied to all branches, not just 14/15 as mentioned in the subject. Michael
Issues with Information_schema.views
Hello The following was tested in a PostgreSQL (16) database. In my opinion queries based on Information_schema.views sometimes give unexpected results. CREATE TABLE Dept(deptno SMALLINT NOT NULL, dname VARCHAR(50) NOT NULL, CONSTRAINT pk_dept PRIMARY KEY (deptno)); CREATE TABLE Emp(empno INTEGER NOT NULL, ename VARCHAR(50) NOT NULL, deptno SMALLINT NOT NULL, CONSTRAINT pk_emp PRIMARY KEY (empno), CONSTRAINT fk_emp_dept FOREIGN KEY (deptno) REFERENCES Dept(deptno) ON UPDATE CASCADE); CREATE VIEW emps AS SELECT * FROM Dept INNER JOIN Emp USING (deptno); UPDATE Emps SET ename=Upper(ename); /*ERROR: cannot update view "emps" DETAIL: Views that do not select from a single table or view are not automatically updatable. HINT: To enable updating the view, provide an INSTEAD OF UPDATE trigger or an unconditional ON UPDATE DO INSTEAD rule.*/ SELECT table_schema AS schema, table_name AS view, is_updatable, is_insertable_into FROM Information_schema.views WHERE table_name='emps'; /*is_updatable=NO and is_insertable_into=NO*/ CREATE OR REPLACE RULE emps_insert AS ON INSERT TO Emps DO INSTEAD NOTHING; /*After that: is_insertable_into=YES*/ CREATE OR REPLACE RULE emps_update AS ON UPDATE TO Emps DO INSTEAD NOTHING; /*After that: is_updatable=NO*/ CREATE OR REPLACE RULE emps_delete AS ON DELETE TO Emps DO INSTEAD NOTHING; /*After that: is_updatable=YES*/ 1. Indeed, now I can execute INSERT/UPDATE/DELETE against the view without getting an error. However, I still cannot change the data in the database through the views. 2. is_updatable=YES only after I add both UPDATE and DELETE DO INSTEAD NOTHING rules. My question is: are 1 and 2 the expected behaviour or is there a mistake in the implementation of the information_schema view? Best regards Erki Eessaar
Re: Is this a problem in GenericXLogFinish()?
On Fri, Oct 27, 2023 at 5:45 AM Michael Paquier wrote: > > On Thu, Oct 26, 2023 at 09:40:09AM -0400, Robert Haas wrote: > > Because of this, it is possible for bucketbuf, prevbuf, and wbuf to be > > the same (your first scenario) but the second scenario you mention > > (nextbuf == wbuf) should be impossible. > > Okay.. > > > It seems to me that maybe we shouldn't even be registering wbuf or > > doing anything at all to it if there are no tuples that need moving. > > That would also require some adjustment of the redo routine. > > Hmm. So my question is: do we need the cleanup lock on the write > buffer even if there are no tuples, and even if primary bucket and the > write bucket are the same? > Yes, we need it to exclude any concurrent in-progress scans that could return incorrect tuples during bucket squeeze operation. -- With Regards, Amit Kapila.
Re: Issues with Information_schema.views
On Sat, Oct 28, 2023 at 5:27 PM Erki Eessaar wrote: > > Hello > > > /*After that: is_updatable=YES*/ > > 1. Indeed, now I can execute INSERT/UPDATE/DELETE against the view without > getting an error. However, I still cannot change the data in the database > through the views. https://www.postgresql.org/docs/current/sql-createview.html " A more complex view that does not satisfy all these conditions is read-only by default: the system will not allow an insert, update, or delete on the view. You can get the effect of an updatable view by creating INSTEAD OF triggers on the view, which must convert attempted inserts, etc. on the view into appropriate actions on other tables. For more information see CREATE TRIGGER. Another possibility is to create rules (see CREATE RULE), but in practice triggers are easier to understand and use correctly. " You CAN get the effect of an updateable view. But you need to make the rule/triggers correct. the following RULE can get the expected result. CREATE OR REPLACE RULE emps_update AS ON UPDATE TO Emps DO INSTEAD UPDATE emp SET empno = NEW.empno, ename = NEW.ename, deptno = NEW.deptno; you can also look at src/test/regress/sql/triggers.sql, src/test/regress/sql/rules.sql for more test cases.
Re: Is this a problem in GenericXLogFinish()?
On Sat, Oct 28, 2023 at 03:45:13PM +0530, Amit Kapila wrote:
> Yes, we need it to exclude any concurrent in-progress scans that could
> return incorrect tuples during bucket squeeze operation.
Thanks. So I assume that we should just set REGBUF_NO_CHANGE when the
primary and write buffers are the same and there are no tuples to
move. Say with something like the attached?
--
Michael
diff --git a/src/backend/access/hash/hashovfl.c b/src/backend/access/hash/hashovfl.c
index 9d1ff20b92..9928068179 100644
--- a/src/backend/access/hash/hashovfl.c
+++ b/src/backend/access/hash/hashovfl.c
@@ -647,6 +647,7 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf,
xl_hash_squeeze_page xlrec;
XLogRecPtr recptr;
int i;
+ int wbuf_flags;
xlrec.prevblkno = prevblkno;
xlrec.nextblkno = nextblkno;
@@ -668,7 +669,15 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf,
XLogRegisterBuffer(0, bucketbuf, flags);
}
- XLogRegisterBuffer(1, wbuf, REGBUF_STANDARD);
+ /*
+ * If the bucket buffer and the primary buffer are the same, a cleanup
+ * lock is still required on the write buffer even if there no tuples
+ * to move, so it needs to be registered in this case.
+ */
+ wbuf_flags = REGBUF_STANDARD;
+ if (xlrec.is_prim_bucket_same_wrt && xlrec.ntups == 0)
+ wbuf_flags |= REGBUF_NO_CHANGE;
+ XLogRegisterBuffer(1, wbuf, wbuf_flags);
if (xlrec.ntups > 0)
{
XLogRegisterBufData(1, (char *) itup_offsets,
signature.asc
Description: PGP signature
Re: maybe a type_sanity. sql bug
On Fri, Oct 27, 2023 at 09:44:30PM -0400, Tom Lane wrote: > Anyway, we should fix this if only for clarity's sake. > I do not feel a need to back-patch though. Agreed. Thanks for the commit. -- Michael signature.asc Description: PGP signature
Re: Enderbury Island disappeared from timezone database
В Fri, 27 Oct 2023 14:00:38 -0400 Tom Lane пишет: > This means that if we substitute Kanton for Enderbury, things > will work fine against tzdata 2021b or later, but will fail in > the reverse way against older tzdata sets. Do we want to > bet that everybody in the world has up-to-date tzdata installed? You are right. When nightly builds came, they showed problems with Pacific/Kanton in Debian 10, 11 and Ubuntu 20.04 (we do not more test ubuntu 18.04 as 5 year support period is ended). I haven't applied 'fix' to rpm-based disitrubutions, because none of them as I'm aware of split tzdata into two packages. > I guess the contract for using --with-system-tzdata is that it's > up to you to maintain that, but still I don't like the odds. > > The alternative I'm wondering about is whether to just summarily > remove the PHOT entry from timezonesets/Default. It's a made-up > zone abbreviation in the first place, and per the above NEWS entry, > there's only a couple dozen people in the world who might even > be candidates to consider using it. It seems highly likely that > nobody would care if we just dropped it from the Default list. > (We could keep the Pacific.txt entry, although re-pointing it > to Pacific/Kanton seems advisable.) > > regards, tom lane > > -- Victor Wagner
Re: Enderbury Island disappeared from timezone database
Victor Wagner writes: > Tom Lane пишет: >> This means that if we substitute Kanton for Enderbury, things >> will work fine against tzdata 2021b or later, but will fail in >> the reverse way against older tzdata sets. Do we want to >> bet that everybody in the world has up-to-date tzdata installed? > You are right. When nightly builds came, they showed problems with > Pacific/Kanton in > Debian 10, 11 and Ubuntu 20.04 (we do not more test ubuntu 18.04 as 5 > year support period is ended). OK. Let's just remove the PHOT entry then. It's not like it's hard to make a custom abbreviation list, in case there's actually somebody out there who needs it. regards, tom lane
Re: run pgindent on a regular basis / scripted manner
On 2023-08-12 Sa 11:57, Andrew Dunstan wrote: On 2023-08-11 Fr 19:17, Tom Lane wrote: Peter Geoghegan writes: I'm starting to have doubts about this policy. There have now been quite a few follow-up "fixes" to indentation issues that koel complained about. None of these fixups have been included in .git-blame-ignore-revs. If things continue like this then "git blame" is bound to become much less usable over time. FWIW, I'm much more optimistic than that. I think what we're seeing is just the predictable result of not all committers having yet incorporated "pgindent it before committing" into their workflow. The need for followup fixes should diminish as people start doing that. If you want to hurry things along, peer pressure on committers who clearly aren't bothering is the solution. Yeah, part of the point of creating koel was to give committers a bit of a nudge in that direction. With a git pre-commit hook it's pretty painless. Based on recent experience, where a lot koel's recent complaints seem to be about comments, I'd like to suggest a modest adjustment. First, we should provide a mode of pgindent that doesn't reflow comments. pg_bsd_indent has a flag for this (-nfcb), so this should be relatively simple. Second, koel could use that mode, so that it wouldn't complain about comments it thinks need to be reflowed. Of course, we'd fix these up with our regular pgindent runs. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: run pgindent on a regular basis / scripted manner
Andrew Dunstan writes: > Based on recent experience, where a lot koel's recent complaints seem to > be about comments, I'd like to suggest a modest adjustment. > First, we should provide a mode of pgindent that doesn't reflow > comments. pg_bsd_indent has a flag for this (-nfcb), so this should be > relatively simple. Second, koel could use that mode, so that it > wouldn't complain about comments it thinks need to be reflowed. Of > course, we'd fix these up with our regular pgindent runs. Seems like a bit of a kluge. Maybe it's the right thing to do, but I don't think we have enough data points yet to be confident that it'd meaningfully reduce the number of breakages. On a more abstract level: the point of trying to maintain indent cleanliness is so that if you modify a file and then want to run pgindent on your own changes, you don't get incidental changes elsewhere in the file. This solution would break that, so I'm not sure it isn't throwing the baby out with the bathwater. regards, tom lane
Re: remove useless comments
On Wed, Aug 10, 2022 at 12:24:02AM +0800, Junwang Zhao wrote: > Fair enough, the rephased version of the comments is in the attachment, > please take a look. > > --- a/src/backend/postmaster/postmaster.c > +++ b/src/backend/postmaster/postmaster.c > @@ -1583,8 +1583,8 @@ getInstallationPaths(const char *argv0) > FreeDir(pdir); > > /* > - * XXX is it worth similarly checking the share/ directory? If the lib/ > - * directory is there, then share/ probably is too. > + * It's not worth checking the share/ directory. If the lib/ directory > + * is there, then share/ probably is too. > */ > } Patch applied to master. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: Why is DEFAULT_FDW_TUPLE_COST so insanely low?
Has anything been done about this issue?
---
On Wed, Aug 3, 2022 at 02:56:12AM +1200, David Rowley wrote:
> Over on [1] I was complaining that I thought DEFAULT_FDW_TUPLE_COST,
> which is defined as 0.01 was unrealistically low.
>
> For comparison, cpu_tuple_cost, something we probably expect to be in
> a CPU cache is also 0.01. We've defined DEFAULT_PARALLEL_TUPLE_COST
> to be 0.1, which is 10x cpu_tuple_cost. That's coming from a shared
> memory segment. So why do we think DEFAULT_FDW_TUPLE_COST should be
> the same as cpu_tuple_cost when that's probably pulling a tuple from
> some remote server over some (possibly slow) network?
>
> I did a little experiment in the attached .sql file and did some maths
> to try to figure out what it's really likely to be costing us. I tried
> this with and without the attached hack to have the planner not
> consider remote grouping just to see how much slower pulling a million
> tuples through the FDW would cost.
>
> I setup a loopback server on localhost (which has about the lowest
> possible network latency) and found the patched query to the foreign
> server took:
>
> Execution Time: 530.000 ms
>
> This is pulling all million tuples over and doing the aggregate locally.
>
> Unpatched, the query took:
>
> Execution Time: 35.334 ms
>
> so about 15x faster.
>
> If I take the seqscan cost for querying the local table, which is
> 14425.00 multiply that by 15 (the extra time it took to pull the 1
> million tuples) then divide by 1 million to get the extra cost per
> tuple, then that comes to about 0.216. So that says
> DEFAULT_FDW_TUPLE_COST is about 21x lower than it should be.
>
> I tried cranking DEFAULT_FDW_TUPLE_COST up to 0.5 to see what plans
> would change in the postgres_fdw regression tests and quite a number
> changed. Many seem to be pushing the sorts down to the remote server
> where they were being done locally before. A few others just seem
> weird. For example, the first one seems to be blindly adding a remote
> sort when it does no good. I think it would take quite a bit of study
> with a debugger to figure out what's going on with many of these.
>
> Does anyone have any ideas why DEFAULT_FDW_TUPLE_COST was set so low?
>
> Does anyone object to it being set to something more realistic?
>
> David
>
> [1]
> https://www.postgresql.org/message-id/CAApHDvpXiXLxg4TsA8P_4etnuGQqAAbHWEOM4hGe=dcaxmi...@mail.gmail.com
> diff --git a/src/backend/optimizer/plan/planner.c
> b/src/backend/optimizer/plan/planner.c
> index 64632db73c..b4e3b91d7f 100644
> --- a/src/backend/optimizer/plan/planner.c
> +++ b/src/backend/optimizer/plan/planner.c
> @@ -3921,7 +3921,7 @@ create_ordinary_grouping_paths(PlannerInfo *root,
> RelOptInfo *input_rel,
>* If there is an FDW that's responsible for all baserels of the query,
>* let it consider adding ForeignPaths.
>*/
> - if (grouped_rel->fdwroutine &&
> + if (0 && grouped_rel->fdwroutine &&
> grouped_rel->fdwroutine->GetForeignUpperPaths)
> grouped_rel->fdwroutine->GetForeignUpperPaths(root,
> UPPERREL_GROUP_AGG,
>
> input_rel, grouped_rel,
> ALTER SYSTEM SET max_parallel_workers_per_gather = 0;
> SELECT pg_reload_conf();
>
> CREATE EXTENSION postgres_fdw;
> CREATE EXTENSION pg_prewarm;
>
>
> DO $d$
> BEGIN
> EXECUTE $$CREATE SERVER loopback FOREIGN DATA WRAPPER postgres_fdw
> OPTIONS (dbname '$$||current_database()||$$',
> port '$$||current_setting('port')||$$'
> )$$;
> END;
> $d$;
>
> CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
>
> CREATE TABLE public.t (a INT);
> INSERT INTO t SELECT x FROM generate_series(1,100) x;
> VACUUM FREEZE ANALYZE t;
> SELECT pg_prewarm('t');
>
> CREATE FOREIGN TABLE ft (
> a INT
> ) SERVER loopback OPTIONS (schema_name 'public', table_name 't');
>
> EXPLAIN (ANALYZE) SELECT COUNT(*) FROM ft;
> EXPLAIN (ANALYZE) SELECT COUNT(*) FROM t;
>
> DROP FOREIGN TABLE ft;
> DROP TABLE t;
> DROP SERVER loopback CASCADE;
> ALTER SYSTEM RESET max_parallel_workers_per_gather;
> SELECT pg_reload_conf();
> --- "expected\\postgres_fdw.out" 2022-08-03 01:34:42.806967000 +1200
> +++ "results\\postgres_fdw.out" 2022-08-03 02:33:40.719712900 +1200
> @@ -2164,8 +2164,8 @@
> -- unsafe conditions on one side (c8 has a UDT), not pushed down.
> EXPLAIN (VERBOSE, COSTS OFF)
> SELECT t1.c1, t2.c1 FROM ft1 t1 LEFT JOIN ft2 t2 ON (t1.c1 = t2.c1) WHERE
> t1.c8 = 'foo' ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10;
> - QUERY PLAN
> --
> + QUERY PLAN
>
Re: COPY TO (FREEZE)?
On Tue, Aug 2, 2022 at 05:17:35PM +0900, Kyotaro Horiguchi wrote: > At Tue, 2 Aug 2022 14:17:46 +0800, Julien Rouhaud wrote > in > > Hi, > > > > On Tue, Aug 02, 2022 at 01:30:46PM +0900, Kyotaro Horiguchi wrote: > > > I noticed that COPY TO accepts FREEZE option but it is pointless. > > > > > > Don't we reject that option as the first-attached does? > > > > I agree that we should reject it, +1 for the patch. > > Thanks for looking it! > > > > By the way, most of the invalid option combinations for COPY are > > > marked as ERRCODE_FEATURE_NOT_SUPPORTED. I looks to me saying that > > > "that feature is theoretically possible or actually realized > > > elsewhere, but impossible now or here". > > > > > > If it is correct, aren't they better be ERRCODE_INVALID_PARAMETER_VALUE? > > > The > > > code is being used for similar messages "unrecognized parameter " > > > and > > > "parameter specified more than once" (or some others?). At least a > > > quote string longer than a single character seems like to fit > > > INVALID_PARAMETER_VALUE. (I believe we don't mean to support > > > multicharacter > > > (or even multibyte) escape/quote character anddelimiter). That being > > > said, > > > I'm not sure if the change will be worth the trouble. > > > > I also feel weird about it. I raised the same point recently about COPY > > FROM + > > HEADER MATCH (1), and at that time there wasn't a real consensus on the way > > to > > go, just keep the things consistent. I'm +0.5 on that patch for the same > > reason as back then. My only concern is that it can in theory break things > > if > > you rely on the current sqlstate, but given the errors I don't think it's > > really a problem. > > Exactly. That is the exact reason for my to say "I'm not sure if..". > > > [1]: > > https://www.postgresql.org/message-id/flat/20220614091319.jk4he5migtpwyd7r%40jrouhaud#b18bf3705fb9f69d0112b6febf0fa1be > > > Maybe that's just me but I understand "not supported" as "this makes > > sense, but this is currently a limitation that might be lifted > > later". > > FWIW I understand it the same way. I would like to apply the attached patch to master. Looking at your adjustments for ERRCODE_FEATURE_NOT_SUPPORTED to ERRCODE_INVALID_PARAMETER_VALUE, I only changed the cases where it would be illogical to implement the feature, not just that we have no intention of implementing the feature. I read "invalid" as "illogical". -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you. diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index d94e3cacfc..cc7d797159 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1119,6 +1119,10 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g destination, because all data must pass through the client/server connection. For large amounts of data the SQL command might be preferable. +Also, because of this pass-through method, \copy +... from in CSV mode will erroneously +treat a \. data value alone on a line as an +end-of-input marker. diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c index b3cc3d9a29..8d6ce4cedd 100644 --- a/src/bin/psql/copy.c +++ b/src/bin/psql/copy.c @@ -627,6 +627,7 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary, PGresult **res) * This code erroneously assumes '\.' on a line alone * inside a quoted CSV string terminates the \copy. * https://www.postgresql.org/message-id/e1tdnvq-0001ju...@wrigleys.postgresql.org + * https://www.postgresql.org/message-id/bfcd57e4-8f23-4c3e-a5db-2571d0920...@beta.fastmail.com */ if ((linelen == 3 && memcmp(fgresult, "\\.\n", 3) == 0) || (linelen == 4 && memcmp(fgresult, "\\.\r\n", 4) == 0))
Re: COPY TO (FREEZE)?
On Sat, Oct 28, 2023 at 08:38:26PM -0400, Bruce Momjian wrote:
> I would like to apply the attached patch to master. Looking at your
> adjustments for ERRCODE_FEATURE_NOT_SUPPORTED to
> ERRCODE_INVALID_PARAMETER_VALUE, I only changed the cases where it would
> be illogical to implement the feature, not just that we have no
> intention of implementing the feature. I read "invalid" as "illogical".
My apologies, wrong patch attached, right one attached now.
--
Bruce Momjian https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index d12ba96497..82b65543c3 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -224,6 +224,7 @@ COPY { table_name [ ( COPY FREEZE on
a partitioned table.
+ This option is allowed only in COPY FROM.
Note that all other sessions will immediately be able to see the data
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index c5d7d78645..4c23c133e1 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -617,7 +617,7 @@ ProcessCopyOptions(ParseState *pstate,
/* Only single-byte delimiter strings are supported. */
if (strlen(opts_out->delim) != 1)
ereport(ERROR,
-(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("COPY delimiter must be a single one-byte character")));
/* Disallow end-of-line characters */
@@ -728,16 +728,22 @@ ProcessCopyOptions(ParseState *pstate,
/* Don't allow the delimiter to appear in the null string. */
if (strchr(opts_out->null_print, opts_out->delim[0]) != NULL)
ereport(ERROR,
-(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("COPY delimiter must not appear in the NULL specification")));
/* Don't allow the CSV quote char to appear in the null string. */
if (opts_out->csv_mode &&
strchr(opts_out->null_print, opts_out->quote[0]) != NULL)
ereport(ERROR,
-(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("CSV quote character must not appear in the NULL specification")));
+ /* Check freeze */
+ if (opts_out->freeze && !is_from)
+ ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY freeze only available using COPY FROM")));
+
if (opts_out->default_print)
{
if (!is_from)
Re: COPY TO (FREEZE)?
Bruce Momjian writes:
> My apologies, wrong patch attached, right one attached now.
I think this one is fine as-is:
/* Only single-byte delimiter strings are supported. */
if (strlen(opts_out->delim) != 1)
ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("COPY delimiter must be a single
one-byte character")));
While we have good implementation reasons for this restriction,
there's nothing illogical about wanting the delimiter to be more
general. It's particularly silly, from an end-user's standpoint,
that for example 'é' is an allowed delimiter in LATIN1 encoding
but not when the server is using UTF8. So I don't see how the
distinction you presented justifies this change.
+ if (opts_out->freeze && !is_from)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+errmsg("COPY freeze only available using COPY
FROM")));
Not thrilled by the wording here. I don't like the fact that the
keyword FREEZE isn't capitalized, and I think you omitted too many
words for intelligibility to be preserved. Notably, all the adjacent
examples use "must" or "must not", and this decides that that can be
omitted.
I realize that you probably modeled the non-capitalization on nearby
messages like "COPY delimiter", but there's a difference IMO:
"delimiter" can be read as an English noun, but it's hard to read
"freeze" as a noun.
How about, say,
errmsg("COPY FREEZE must not be used in COPY TO")));
or perhaps that's redundant and we could write
errmsg("FREEZE option must not be used in COPY TO")));
regards, tom lane
Re: COPY TO (FREEZE)?
On Sat, Oct 28, 2023 at 09:39:53PM -0400, Tom Lane wrote:
> Bruce Momjian writes:
> > My apologies, wrong patch attached, right one attached now.
>
> I think this one is fine as-is:
>
> /* Only single-byte delimiter strings are supported. */
> if (strlen(opts_out->delim) != 1)
> ereport(ERROR,
> - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>errmsg("COPY delimiter must be a single
> one-byte character")));
>
> While we have good implementation reasons for this restriction,
> there's nothing illogical about wanting the delimiter to be more
> general. It's particularly silly, from an end-user's standpoint,
> that for example 'é' is an allowed delimiter in LATIN1 encoding
> but not when the server is using UTF8. So I don't see how the
> distinction you presented justifies this change.
Agreed, my mistake.
> + if (opts_out->freeze && !is_from)
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("COPY freeze only available using COPY
> FROM")));
>
> Not thrilled by the wording here. I don't like the fact that the
> keyword FREEZE isn't capitalized, and I think you omitted too many
> words for intelligibility to be preserved. Notably, all the adjacent
> examples use "must" or "must not", and this decides that that can be
> omitted.
I think it is modeled after:
errmsg("COPY force null only available using COPY FROM")));
> I realize that you probably modeled the non-capitalization on nearby
> messages like "COPY delimiter", but there's a difference IMO:
> "delimiter" can be read as an English noun, but it's hard to read
> "freeze" as a noun.
>
> How about, say,
>
> errmsg("COPY FREEZE must not be used in COPY TO")));
>
> or perhaps that's redundant and we could write
>
> errmsg("FREEZE option must not be used in COPY TO")));
I now have:
errmsg("COPY FREEZE mode only available using COPY FROM")));
Updated patch attached.
--
Bruce Momjian https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index d12ba96497..82b65543c3 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -224,6 +224,7 @@ COPY { table_name [ ( COPY FREEZE on
a partitioned table.
+ This option is allowed only in COPY FROM.
Note that all other sessions will immediately be able to see the data
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index c5d7d78645..92558b6bb0 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -728,16 +728,22 @@ ProcessCopyOptions(ParseState *pstate,
/* Don't allow the delimiter to appear in the null string. */
if (strchr(opts_out->null_print, opts_out->delim[0]) != NULL)
ereport(ERROR,
-(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("COPY delimiter must not appear in the NULL specification")));
/* Don't allow the CSV quote char to appear in the null string. */
if (opts_out->csv_mode &&
strchr(opts_out->null_print, opts_out->quote[0]) != NULL)
ereport(ERROR,
-(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("CSV quote character must not appear in the NULL specification")));
+ /* Check freeze */
+ if (opts_out->freeze && !is_from)
+ ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY FREEZE mode only available using COPY FROM")));
+
if (opts_out->default_print)
{
if (!is_from)
Re: COPY TO (FREEZE)?
Bruce Momjian writes:
> On Sat, Oct 28, 2023 at 09:39:53PM -0400, Tom Lane wrote:
>> Not thrilled by the wording here.
> I think it is modeled after:
> errmsg("COPY force null only available using COPY FROM")));
Well, now that you bring it up, that's no sterling example of
clear writing either. Maybe change that while we're at it,
say to "FORCE NULL option must not be used in COPY TO"?
(Also, has it got the right ERRCODE?)
regards, tom lane
Re: COPY TO (FREEZE)?
On Sat, Oct 28, 2023 at 09:54:05PM -0400, Tom Lane wrote:
> Bruce Momjian writes:
> > On Sat, Oct 28, 2023 at 09:39:53PM -0400, Tom Lane wrote:
> >> Not thrilled by the wording here.
>
> > I think it is modeled after:
>
> > errmsg("COPY force null only available using COPY FROM")));
>
> Well, now that you bring it up, that's no sterling example of
> clear writing either. Maybe change that while we're at it,
> say to "FORCE NULL option must not be used in COPY TO"?
I used:
"COPY FREEZE mode cannot be used with COPY FROM"
and adjusted the others.
> (Also, has it got the right ERRCODE?)
Fixed, and the other cases too. Patch attached.
--
Bruce Momjian https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index d12ba96497..82b65543c3 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -224,6 +224,7 @@ COPY { table_name [ ( COPY FREEZE on
a partitioned table.
+ This option is allowed only in COPY FROM.
Note that all other sessions will immediately be able to see the data
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index c5d7d78645..bda3f8b9f9 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -711,8 +711,8 @@ ProcessCopyOptions(ParseState *pstate,
errmsg("COPY force not null available only in CSV mode")));
if (opts_out->force_notnull != NIL && !is_from)
ereport(ERROR,
-(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("COPY force not null only available using COPY FROM")));
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY force not null cannot be used with COPY FROM")));
/* Check force_null */
if (!opts_out->csv_mode && opts_out->force_null != NIL)
@@ -722,22 +722,28 @@ ProcessCopyOptions(ParseState *pstate,
if (opts_out->force_null != NIL && !is_from)
ereport(ERROR,
-(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("COPY force null only available using COPY FROM")));
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY force null cannot be used with COPY FROM")));
/* Don't allow the delimiter to appear in the null string. */
if (strchr(opts_out->null_print, opts_out->delim[0]) != NULL)
ereport(ERROR,
-(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("COPY delimiter must not appear in the NULL specification")));
/* Don't allow the CSV quote char to appear in the null string. */
if (opts_out->csv_mode &&
strchr(opts_out->null_print, opts_out->quote[0]) != NULL)
ereport(ERROR,
-(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("CSV quote character must not appear in the NULL specification")));
+ /* Check freeze */
+ if (opts_out->freeze && !is_from)
+ ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY FREEZE mode cannot be used with COPY FROM")));
+
if (opts_out->default_print)
{
if (!is_from)
RE: pg_upgrade test failure
Dear Andres, While tracking BF failures related with pg_ugprade, I found the same failure has still happened [1] - [4]. According to the log, the output directory was remained even after the successful upgrade [5]. I analyzed and attached the fix patch, and below is my analysis... how do you think? = lstat() seemed fail while doing the second try of rmtree(). This error message is output from get_dirent_type(). Apart from pgunlink(), get_dirent_type() does not have an retry mechanism when lstat()->_pglstat64() detects STATUS_DELETE_PENDING. Therefore, I think rmtree() may not wait the file until it would be really removed, if the status is deceted in the get_dirent_type(). One solution is to retry stat() or lstat() even in get_dirent_type(), like attached. [1]: 2023-07-21 02:21:53 https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&dt=2023-07-21%2002%3A21%3A53 [2]: 2023-10-21 13:39:15 https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&dt=2023-10-21%2013%3A39%3A15 [3]: 2023-10-23 09:03:07 https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&dt=2023-10-23%2009%3A03%3A07 [4]: 2023-10-27 23:06:17 https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&dt=2023-10-27%2023%3A06%3A17 [5] ``` ... *Clusters are compatible* pg_upgrade: warning: could not remove directory "C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql.build/testrun/pg_upgrade/002_pg_upgrade/data/t_002_pg_upgrade_new_node_data/pgdata/pg_upgrade_output.d/20231027T234552.867/log": Directory not empty pg_upgrade: warning: could not remove directory "C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql.build/testrun/pg_upgrade/002_pg_upgrade/data/t_002_pg_upgrade_new_node_data/pgdata/pg_upgrade_output.d/20231027T234552.867": Directory not empty pg_upgrade: warning: could not stat file "C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql.build/testrun/pg_upgrade/002_pg_upgrade/data/t_002_pg_upgrade_new_node_data/pgdata/pg_upgrade_output.d/20231027T234552.867/log/pg_upgrade_internal.log": No such file or directory pg_upgrade: warning: could not remove directory "C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql.build/testrun/pg_upgrade/002_pg_upgrade/data/t_002_pg_upgrade_new_node_data/pgdata/pg_upgrade_output.d/20231027T234552.867/log": Directory not empty pg_upgrade: warning: could not remove directory "C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql.build/testrun/pg_upgrade/002_pg_upgrade/data/t_002_pg_upgrade_new_node_data/pgdata/pg_upgrade_output.d/20231027T234552.867": Directory not empty [23:46:07.585](17.106s) ok 12 - run of pg_upgrade --check for new instance [23:46:07.587](0.002s) not ok 13 - pg_upgrade_output.d/ removed after pg_upgrade --check success ... ``` Best Regards, Hayato Kuroda FUJITSU LIMITED 0001-Retry-stat-and-lstat-even-in-get_dirent_type.patch Description: 0001-Retry-stat-and-lstat-even-in-get_dirent_type.patch
Re: COPY TO (FREEZE)?
Bruce Momjian 于2023年10月29日 周日10:04写道：
> On Sat, Oct 28, 2023 at 09:54:05PM -0400, Tom Lane wrote:
> > Bruce Momjian writes:
> > > On Sat, Oct 28, 2023 at 09:39:53PM -0400, Tom Lane wrote:
> > >> Not thrilled by the wording here.
> >
> > > I think it is modeled after:
> >
> > > errmsg("COPY force null only available using COPY FROM")));
> >
> > Well, now that you bring it up, that's no sterling example of
> > clear writing either. Maybe change that while we're at it,
> > say to "FORCE NULL option must not be used in COPY TO"?
>
> I used:
>
> "COPY FREEZE mode cannot be used with COPY FROM"
>
> and adjusted the others.
>
> > (Also, has it got the right ERRCODE?)
>
> Fixed, and the other cases too. Patch attached.
>
> --
> Bruce Momjian https://momjian.us
> EDB https://enterprisedb.com
>
> Only you can decide what is important to you.
errmsg("COPY force not null only available using COPY FROM")));
>
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>
> + errmsg("COPY force not null cannot be used with COPY FROM")));
>
>
cannot -> can ?
>
Re: COPY TO (FREEZE)?
Mingli Zhang 于2023年10月29日 周日14:35写道：
>
>
> Bruce Momjian 于2023年10月29日 周日10:04写道：
>
>> On Sat, Oct 28, 2023 at 09:54:05PM -0400, Tom Lane wrote:
>> > Bruce Momjian writes:
>> > > On Sat, Oct 28, 2023 at 09:39:53PM -0400, Tom Lane wrote:
>> > >> Not thrilled by the wording here.
>> >
>> > > I think it is modeled after:
>> >
>> > > errmsg("COPY force null only available using COPY FROM")));
>> >
>> > Well, now that you bring it up, that's no sterling example of
>> > clear writing either. Maybe change that while we're at it,
>> > say to "FORCE NULL option must not be used in COPY TO"?
>>
>> I used:
>>
>> "COPY FREEZE mode cannot be used with COPY FROM"
>>
>> and adjusted the others.
>>
>> > (Also, has it got the right ERRCODE?)
>>
>> Fixed, and the other cases too. Patch attached.
>>
>> --
>> Bruce Momjian https://momjian.us
>> EDB https://enterprisedb.com
>>
>> Only you can decide what is important to you.
>
>
> errmsg("COPY force not null only available using COPY FROM")));
>>
>> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>>
>> + errmsg("COPY force not null cannot be used with COPY FROM")));
>>
>>
> cannot -> can ?
>
I guess you want to write “cannot be used with COPY TO”
>
