| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Tatsuro Yamada <yamatattsu(at)gmail(dot)com> |
| Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Subject: | Re: [PATCH] psql: add \dcs to list all constraints |
| Date: | 2026-01-15 07:20:14 |
| Message-ID: | E24F8467-B5F2-462B-AC3F-DD4321EC0418@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Jan 15, 2026, at 14:32, Tatsuro Yamada <yamatattsu(at)gmail(dot)com> 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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Shinya Kato | 2026-01-15 07:23:46 | Re: file_fdw: Support multi-line HEADER option. |
| Previous Message | Shinya Kato | 2026-01-15 07:14:35 | Re: file_fdw: Support multi-line HEADER option. |