On 2025/06/26 3:45, Álvaro Herrera wrote:
On 2025-Jun-25, Álvaro Herrera wrote:

Ah, thanks for the test case.  Yeah, I removed one 'if' condition too
many from Jian's patch.  We just need to test the constraint name for
nullness and then things seem to work:

One more thing was missing, which I noticed as I added the tests.
Apparently the COMMENT objects can end up in any section of the dump,
but for comments on valid constraints, this is not what we want -- they
should go into the PRE_DATA section only.  When running the tests with
the code still putting them in SECTION_NONE, apparently pg_dump would
randomly put them either here or there, so the tests would fail
intermittently.  Or at least that's what I think happened.  Once I made
them be in SECTION_PRE_DATA, that problem disappeared.

Anyway, here's what I intend to push tomorrow, unless further problems
are found.

Thanks for updating the patch!

I noticed a small inconsistency in the output of pg_dump when handling
comments for COMMENT commands on not-null constraints. For example,
with the following DDL:

-------------
CREATE SCHEMA hoge;
CREATE TABLE hoge.t (i int PRIMARY KEY, j int NOT NULL CHECK (j > 0));
COMMENT ON CONSTRAINT t_pkey ON hoge.t IS 'primary key';
COMMENT ON CONSTRAINT t_j_not_null ON hoge.t IS 'not null';
COMMENT ON CONSTRAINT t_j_check ON hoge.t IS 'check';
-------------

The pg_dump output shows the following comments for COMMENT commands:

-------------
-- Name: CONSTRAINT t_j_not_null ON hoge.t; Type: COMMENT; Schema: hoge; Owner: 
postgres
-- Name: CONSTRAINT t_j_check ON t; Type: COMMENT; Schema: hoge; Owner: postgres
-- Name: CONSTRAINT t_pkey ON t; Type: COMMENT; Schema: hoge; Owner: postgres
-------------

You can see that only comments for the not-null constraint includes
the schema-qualified table name (hoge.t) after "ON". The others just
show "t". It's a very minor issue, but for consistency, it would be
better if all constraint comments used the same format.


+               if (comment != NULL)
+               {
+                       destroyPQExpBuffer(comment);
+                       destroyPQExpBuffer(tag);

The "comment != NULL" check isn't needed here, since destroyPQExpBuffer()
already handles null safely.


Since valid not-null constraints are dumped in the pre-data section,
the following change seems necessary in pg_dump.sgml.

           statistics for indexes, and constraints other than validated check
-          constraints.
+          and not-null constraints.
           Pre-data items include all other data definition items.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation



Reply via email to