Re: pg14 psql broke \d datname.nspname.relname

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Mark Dilger <mark.dilger@enterprisedb.com>, Andres Freund <andres@anarazel.de>, Julien Rouhaud <rjuju123@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Justin Pryzby <pryzby@telsasoft.com>, Stephen Frost <sfrost@snowman.net>, Peter Geoghegan <pg@bowt.ie>
Date: 2022-04-08T02:26:18Z
Lists: pgsql-hackers
On Thu, Apr 7, 2022 at 7:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Mark Dilger <mark.dilger@enterprisedb.com> writes:
> > The patch submitted changes processSQLNamePattern() to return a dot count by reference.  It's up to the caller to decide whether to raise an error.  If you pass in no schemavar, and you get back dotcnt=2, you know it parsed it as a two part pattern, and you can pg_fatal(...) or ereport(ERROR, ...) or whatever.
>
> Well, I'm not telling Robert what to do, but I wouldn't accept that
> API.  It requires duplicative error-handling code at every call site
> and is an open invitation to omitting necessary error checks.
>
> Possibly a better idea is to add an enum argument telling the function
> what to do (parse the whole thing as one name regardless of dots,
> parse as two names if there's a dot, throw error if there's a dot,
> etc etc as needed by existing call sites).  Perhaps some of the
> existing arguments could be merged into such an enum, too.

I hadn't considered that approach, but I don't think it works very
well, because front-end error handling is so inconsistent. From the
patch:

+               pg_log_error("improper relation name (too many dotted
names): %s", pattern);
+               exit(2);

+                       fatal("improper qualified name (too many
dotted names): %s",
+                                 cell->val);

+                       pg_log_error("improper qualified name (too
many dotted names): %s",
+                                                cell->val);
+                       PQfinish(conn);
+                       exit_nicely(1);

+               pg_log_error("improper qualified name (too many dotted
names): %s",
+                                        pattern);
+               termPQExpBuffer(&dbbuf);
+               return false;

Come to think of it, maybe the error text there could stand some
bikeshedding, but AFAICS there's not much to be done about the fact
that one caller wants pg_log_error + exit(2), another wants fatal(), a
third PQfinish(conn) and exit_nicely(), and the last termPQExpBuffer()
and return false.

-- 
Robert Haas
EDB: http://www.enterprisedb.com



Commits

  1. Remove some recently-added pg_dump test cases.

  2. Allow db.schema.table patterns, but complain about random garbage.

  3. Improve error handling of HMAC computations

  4. Factor pattern-construction logic out of processSQLNamePattern.