doc pg_constraint.convalidated column description need update

On Thu, May 1, 2025 at 10:19 PM jian he <jian.universality@gmail.com> wrote:

hi.

catalog.sgml:

<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>convalidated</structfield> <type>bool</type>
</para>
<para>
Has the constraint been validated?
Currently, can be false only for foreign keys and CHECK constraints
</para></entry>
</row>

with NOT NULL NOT VALID,
we need rephrase it to something like:
"Currently, can be false only for foreign keys, CHECK and not-null constraints"
I am also ok with just deleting it.

I actually think this mix of VALID / ENFORCED is a little convoluted
for end users, but I don't think the system catalog documentation is
the best place to work that out, and I see we've also not added this
type of extra information to other columns in the table that have
similar restrictions, so I think removing it is the right move.

As such, attached patch removes the above, and attempts some clean up
of the documentation in ALTER TABLE to better clarify the behavior
around valid/not valid, enforced/not enforced, and how it affects
different constraints, with some additional literal tag formatting
changes.

BTW, I happen to notice a minor issue:
some pg_catalog table column entry descriptions ending with a period,
some didn't.

I don't know if it is intentional, but this seems to be determined by
a rough heuristic on if the description contains a complete
sentence(s) or not, but I would agree there are cases that don't
follow that, and some that could probably be argued either way. If it
were me I'd probably add punctuation to all of the entries, but others
may feel different.

Robert Treat
https://xzilla.net

On Sat, May 3, 2025 at 11:42 PM Robert Treat <rob@xzilla.net> wrote:

As such, attached patch removes the above, and attempts some clean up
of the documentation in ALTER TABLE to better clarify the behavior
around valid/not valid, enforced/not enforced, and how it affects
different constraints, with some additional literal tag formatting
changes.

    <para>
-    Adding an enforced <literal>CHECK</literal> or <literal>NOT NULL</literal>
-    constraint requires scanning the table to verify that existing
rows meet the
-    constraint, but does not require a table rewrite.  If a
<literal>CHECK</literal>
-    constraint is added as <literal>NOT ENFORCED</literal>, the validation will
-    not be performed.
+    Adding a <literal>CHECK</literal> or <literal>NOT NULL</literal>
+    constraint requires scanning the table to verify that existing rows meet
+    the constraint, but does not require a table rewrite.  IF a
+    <literal>CHECK</literal> constraint is added as
+    <literal>NOT ENFORCED</literal>, no verification is performed.
    </para>

"IF" should be "if".
i don't know which one ("validation" or "verification") is more accureate,
but i found this
(https://www.eviltester.com/2018/09/no-verification-validation.html)

here, we should also mention <literal>FOREIGN KEY</literal> constraint?

-      This form validates a foreign key, check, or not-null constraint that was
-      previously created as <literal>NOT VALID</literal>, by scanning the
+      This form validates a <literal>FOREIGN KEY</literal>,
+      <literal>CHECK</literal>, or <literal>NOT NULL</literal> constraint that
+      was previously created as <literal>NOT VALID</literal>, by scanning the
       table to ensure there are no rows for which the constraint is not
-      satisfied.  If the constraint is not enforced, an error is thrown.
+      satisfied.  If the constraint was created as
+      <literal>NOT ENFORCED</literal>, an error is thrown.
I think this changes should be great. original text
"If the constraint is not enforced" can mean "some exists row not
satisfied the constraint
condition" or "the constraint is marked as <literal>NOT ENFORCED</literal>".

On Tue, May 6, 2025 at 4:49 AM jian he <jian.universality@gmail.com> wrote:

On Sat, May 3, 2025 at 11:42 PM Robert Treat <rob@xzilla.net> wrote:

As such, attached patch removes the above, and attempts some clean up
of the documentation in ALTER TABLE to better clarify the behavior
around valid/not valid, enforced/not enforced, and how it affects
different constraints, with some additional literal tag formatting
changes.

<para>
-    Adding an enforced <literal>CHECK</literal> or <literal>NOT NULL</literal>
-    constraint requires scanning the table to verify that existing
rows meet the
-    constraint, but does not require a table rewrite.  If a
<literal>CHECK</literal>
-    constraint is added as <literal>NOT ENFORCED</literal>, the validation will
-    not be performed.
+    Adding a <literal>CHECK</literal> or <literal>NOT NULL</literal>
+    constraint requires scanning the table to verify that existing rows meet
+    the constraint, but does not require a table rewrite.  IF a
+    <literal>CHECK</literal> constraint is added as
+    <literal>NOT ENFORCED</literal>, no verification is performed.
</para>

"IF" should be "if".

Actually "If" I reckon :-)

i don't know which one ("validation" or "verification") is more accureate,
but i found this
(https://www.eviltester.com/2018/09/no-verification-validation.html)

I intentionally chose verification as a callback to the earlier
mention that the table will be scanned to *verify* the rows. I also
want to put some distance between the ENFORCED / NOT ENFORCED language
and NOT VALID / VALIDATE CONSTRAINT commands, which is a separate
feature/functionality on it's own.

here, we should also mention <literal>FOREIGN KEY</literal> constraint?

I didn't think it felt necessary based on other information elsewhere
(and wasn't there before).

-      This form validates a foreign key, check, or not-null constraint that was
-      previously created as <literal>NOT VALID</literal>, by scanning the
+      This form validates a <literal>FOREIGN KEY</literal>,
+      <literal>CHECK</literal>, or <literal>NOT NULL</literal> constraint that
+      was previously created as <literal>NOT VALID</literal>, by scanning the
table to ensure there are no rows for which the constraint is not
-      satisfied.  If the constraint is not enforced, an error is thrown.
+      satisfied.  If the constraint was created as
+      <literal>NOT ENFORCED</literal>, an error is thrown.
I think this changes should be great. original text
"If the constraint is not enforced" can mean "some exists row not
satisfied the constraint
condition" or "the constraint is marked as <literal>NOT ENFORCED</literal>".

Yeah, "not enforced" felt a little loose to me, especially since the
only way a constraint can be NOT ENFORCED is if it was created that
way (not to mention you can't change it later, you have to drop it).

Robert Treat
https://xzilla.net

On 03.05.25 17:42, Robert Treat wrote:

I actually think this mix of VALID / ENFORCED is a little convoluted
for end users, but I don't think the system catalog documentation is
the best place to work that out, and I see we've also not added this
type of extra information to other columns in the table that have
similar restrictions, so I think removing it is the right move.

As such, attached patch removes the above, and attempts some clean up
of the documentation in ALTER TABLE to better clarify the behavior
around valid/not valid, enforced/not enforced, and how it affects
different constraints, with some additional literal tag formatting
changes.

I committed the catalog.sgml change. I don't agree with some of the
other changes, such as changing foreign key to >FOREIGN KEY< etc. It
was hard to tell if there were any other changes in there that could be
considered separately from that.

On Wed, Jun 4, 2025 at 9:37 AM Peter Eisentraut <peter@eisentraut.org> wrote:

On 03.05.25 17:42, Robert Treat wrote:

I actually think this mix of VALID / ENFORCED is a little convoluted
for end users, but I don't think the system catalog documentation is
the best place to work that out, and I see we've also not added this
type of extra information to other columns in the table that have
similar restrictions, so I think removing it is the right move.

As such, attached patch removes the above, and attempts some clean up
of the documentation in ALTER TABLE to better clarify the behavior
around valid/not valid, enforced/not enforced, and how it affects
different constraints, with some additional literal tag formatting
changes.

I committed the catalog.sgml change. I don't agree with some of the
other changes, such as changing foreign key to >FOREIGN KEY< etc. It
was hard to tell if there were any other changes in there that could be
considered separately from that.

I think some of those changes are worth a second stab, so here is an
updated patch which removes the ancillary tagging and corresponding
line wrappings and focuses just on the wording/grammer improvements;
hopefully it will be easier to make sense of.

Robert Treat
https://xzilla.net

On Thu, Jun 5, 2025 at 4:47 AM Robert Treat <rob@xzilla.net> wrote:

I think some of those changes are worth a second stab, so here is an
updated patch which removes the ancillary tagging and corresponding
line wrappings and focuses just on the wording/grammer improvements;
hopefully it will be easier to make sense of.

hi.

    <para>
-    Scanning a large table to verify a new foreign key or check constraint
+    Scanning a large table to verify new foreign key, check, or not
null constraints
     can take a long time, and other updates to the table are locked out
     until the <command>ALTER TABLE ADD CONSTRAINT</command> command is
     committed.  The main purpose of the <literal>NOT VALID</literal>

here, it should be "not-null constraints"?
Other than that, it looks good to me

On Mon, Jun 9, 2025 at 12:00 AM jian he <jian.universality@gmail.com> wrote:

On Thu, Jun 5, 2025 at 4:47 AM Robert Treat <rob@xzilla.net> wrote:

I think some of those changes are worth a second stab, so here is an
updated patch which removes the ancillary tagging and corresponding
line wrappings and focuses just on the wording/grammer improvements;
hopefully it will be easier to make sense of.

hi.

<para>
-    Scanning a large table to verify a new foreign key or check constraint
+    Scanning a large table to verify new foreign key, check, or not
null constraints
can take a long time, and other updates to the table are locked out
until the <command>ALTER TABLE ADD CONSTRAINT</command> command is
committed.  The main purpose of the <literal>NOT VALID</literal>

here, it should be "not-null constraints"?
Other than that, it looks good to me

Thanks for taking a look, and yes, I believe you are correct.

Robert Treat
https://xzilla.net

On 09.06.25 16:32, Robert Treat wrote:

On Mon, Jun 9, 2025 at 12:00 AM jian he <jian.universality@gmail.com> wrote:

On Thu, Jun 5, 2025 at 4:47 AM Robert Treat <rob@xzilla.net> wrote:

I think some of those changes are worth a second stab, so here is an
updated patch which removes the ancillary tagging and corresponding
line wrappings and focuses just on the wording/grammer improvements;
hopefully it will be easier to make sense of.

hi.

<para>
-    Scanning a large table to verify a new foreign key or check constraint
+    Scanning a large table to verify new foreign key, check, or not
null constraints
can take a long time, and other updates to the table are locked out
until the <command>ALTER TABLE ADD CONSTRAINT</command> command is
committed.  The main purpose of the <literal>NOT VALID</literal>

here, it should be "not-null constraints"?
Other than that, it looks good to me

Thanks for taking a look, and yes, I believe you are correct.

committed