Re: [PATCH] psql: add \dcs to list all constraints

Hi Jim,

Thanks for your review comments!

On Tue, Jan 13, 2026 at 12:17 AM Jim Jones <jim.jones@uni-muenster.de>
wrote:

> Here a few comments to v2:
>
== listConstraints() ==

It looks like that a WHERE condition can be potentially added to the "if
> (!showAllkinds)" block even if there is no WHERE clause at all. I'm not
> sure if this path is even possible, but perhaps a more defensive
> approach here wouldn't be a bad idea, e.g.
>
> ...
> bool have_where = false;
>
> if (!showSystem && !pattern)
> {
>         appendPQExpBufferStr(&buf,
>                              "WHERE n.nspname <> 'pg_catalog' \n"
>                              "  AND n.nspname <> 'information_schema' \n");
>         have_where = true;
> }
>
> if (!validateSQLNamePattern(&buf, pattern,
>                 have_where, false,
>                 "n.nspname", "cst.conname", NULL,
>                 "pg_catalog.pg_table_is_visible(cst.conrelid)",
>                 &have_where, 3))
> {
>
> if (!showAllkinds)
> {
>         appendPQExpBuffer(&buf, " %s cst.contype in (",
>                           have_where ? "AND" : "WHERE");
> ...
>
> What do you think?
>

Based on the value passed to validateSQLNamePattern(), one or more of
n.nspname, cst.conname, or pg_catalog.pg_table_is_visible(cst.conrelid) is
added to the query string together with either WHERE or AND.
Therefore, I believe there is no case in which the if (!showAllkinds) block
is
reached without an existing WHERE clause.
If you are aware of a specific test case where this can happen, I would
appreciate it if you could share it with me.

For now, my conclusion is that I would like to keep this part as it is. I
apologize
if I have missed something.


== Patch name ==
>
> It'd be better if you format your patch name with the version upfront, e.g.
>
> $ git format-patch -1 -v3
>

Thank you for the suggestion. From now on, I will generate patches using
the options you mentioned.



> I've tried a few more edge cases and so far everything is working as

expected
>
> postgres=# \set ECHO_HIDDEN on
>
> postgres=# \dcs 🐘*
> /******** QUERY *********/
> ...

FROM pg_catalog.pg_constraint cst
>      JOIN pg_catalog.pg_namespace n ON n.oid = cst.connamespace
>      JOIN pg_catalog.pg_class c on c.oid = cst.conrelid
>

Thank you for testing these additional edge cases.
I noticed that when the "+" (verbose option) is not used, the table name is
not needed. In that case, joining the pg_class table is unnecessary.
This will be fixed in the next patch.

Thanks,
Tatsuro Yamada