Re: pg_dump misses comments on NOT NULL constraints

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