Re: [PATCH] psql: add \dcs to list all constraints - Mailing list pgsql-hackers
| From | Chao Li |
|---|---|
| Subject | Re: [PATCH] psql: add \dcs to list all constraints |
| Date | |
| Msg-id | E24F8467-B5F2-462B-AC3F-DD4321EC0418@gmail.com Whole thread Raw |
| In response to | Re: [PATCH] psql: add \dcs to list all constraints (Tatsuro Yamada <yamatattsu@gmail.com>) |
| Responses |
Re: [PATCH] psql: add \dcs to list all constraints
|
| List | pgsql-hackers |
> On Jan 15, 2026, at 14:32, Tatsuro Yamada <yamatattsu@gmail.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() forvalidation. 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/
pgsql-hackers by date: