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

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Peter Geoghegan <pg(at)bowt(dot)ie>
Subject: Re: pg14 psql broke \d datname.nspname.relname
Date: 2022-04-08 02:26:18
Message-ID: CA+Tgmoa8-QM_RMrCJKwMzS7adF3uNbvBjujGknL1rd2jQa_d7Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Apr 7, 2022 at 7:41 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2022-04-08 02:34:17 Re: Showing I/O timings spent reading/writing temp buffers in EXPLAIN
Previous Message Robert Haas 2022-04-08 02:19:35 Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]