Hi Jim, Thanks for your review comments!
On Tue, Jan 13, 2026 at 12:17 AM Jim Jones <[email protected]> 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
