> On Jan 15, 2026, at 14:32, Tatsuro Yamada <[email protected]> wrote:
> 
> Hi Tom and Jim,
> 
> >The next patch will include the following changes:
> >
> >- Rename the command from \dcs to \dCN (proposed by Tom. Thanks!)
> >- Join pg_class in the query only when the "+" option is used 
> >  (identified through Jim's additional testing. Thanks!)
> 
> I have prepared a new patch and am sending it here.
> Please find the attached file.
> 
> 
> >Note: The following two points, which were discussed in the previous email, 
> >will be addressed once the discussion is settled:
> >
> >- Changing the column order displayed by the \dCN command
> >- Adding domain constraints to the list of displayed items
> 
> I have shared my thoughts on the two points raised by Tom in the email 
> below, and I would appreciate your comments.
> 
>     
> https://www.postgresql.org/message-id/CAOKkKFuYfdvpQ7eSYWxB1YrzwVafWm%2B%2BctXBPe_Rb1YqeOKjjA%40mail.gmail.com
> 
> Regards,
> Tatsuro Yamada 
> <v3-0001-Add-list-constraints-meta-command-dCN-on-psql.patch>

Hi Tatsuro-san,

Thanks for the patch. I just reviewed the patch, and I think it is solid 
already, just got a few small comments:

1 - commit message
```
\dCN shows all kind of constraints by using pg_constraint.
```

All kind -> all kinds


2.
```
+                                       switch (cmd[3])
+                                       {
+                                               case '\0':
+                                               case '+':
+                                               case 'S':
+                                               case 'c':
+                                               case 'f':
+                                               case 'n':
+                                               case 'p':
+                                               case 't':
+                                               case 'u':
+                                               case 'e':
+                                               case 'x':
+                                                       success = 
listConstraints(&cmd[3], pattern, show_verbose, show_system);
+                                                       break;
+                                               default:
+                                                       status = 
PSQL_CMD_UNKNOWN;
+                                                       break;
```
and
```
+       if (strlen(contypes) != strspn(contypes, dCN_options))
+       {
+               pg_log_error("\\dCN only takes [%s] as options", dCN_options);
+               return true;
+       }
```

These two checks are redundant. Is it intentional to keep both? It might be 
simpler to rely solely on listConstraints() for validation.

3
```
---- \dCN doesn't show constraints related to domain,
---- since \dD can be used to check them
```

I saw this in the test script, should we mention that in the doc change?

4
```
+        <term><literal>\dCN[cfnptue][Sx+] [ <link 
linkend="app-psql-patterns"><replaceable 
class="parameter">pattern</replaceable></link> ]</literal></term>

```

Would it make sense to add a white space between [cfnptue] and [Sx+]? As you do 
so for the help message.

5
```
+       if (strlen(contypes) != strspn(contypes, dCN_options))
+       {
+               pg_log_error("\\dCN only takes [%s] as options", dCN_options);
+               return true;
+       }
+
+       if (pset.sversion < 180000)
+       {
+               char            sverbuf[32];
+
+               pg_log_error("The server (version %s) does not support this 
meta-command on psql.",
+                                        formatPGVersionNumber(pset.sversion, 
false,
+                                                                               
   sverbuf, sizeof(sverbuf)));
+               return true;
+       }
```

Why return true after pg_log_error?

6
```
+       if (!(showCheck || showForeign || showNotnull || showPrimary || 
showTrigger || showUnique || showExclusion))
+               showAllkinds = true;
```

Nit: this might be simplified as: showAllkinds = !(showCheck || …), then you 
don’t need to initialize showAllkinds as false.


Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Reply via email to