Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt - Mailing list pgsql-hackers

From Álvaro Herrera
Subject Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt
Date
Msg-id 202508282133.uwntzsze7cdl@alvherre.pgsql
Whole thread Raw
In response to misleading error message in ProcessUtilitySlow T_CreateStatsStmt  (jian he <jian.universality@gmail.com>)
Responses Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt
List pgsql-hackers
On 2025-Aug-21, jian he wrote:

>             case T_CreateStatsStmt:
>                 {
>                     Oid            relid;
>                     CreateStatsStmt *stmt = (CreateStatsStmt *) parsetree;
>                     RangeVar   *rel = (RangeVar *) linitial(stmt->relations);
> 
>                     if (!IsA(rel, RangeVar))
>                         ereport(ERROR,
>                                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>                                  errmsg("only a single relation is
> allowed in CREATE STATISTICS")));
>                     relid = RangeVarGetRelid(rel,
> ShareUpdateExclusiveLock, false);
>                     /* Run parse analysis ... */
>                     stmt = transformStatsStmt(relid, stmt, queryString);
>                     address = CreateStatistics(stmt);
>                 }

Yeah, I think there's room to argue that this ereport() is just wrongly
copy-and-pasted from other nearby errors, and that it should have
completely different wording.  BTW another way to cause this is

CREATE STATISTICS alt_stat2 ON a, b FROM xmltable('foo' passing 'bar' columns a text);

I propose to use this report:

ERROR:  cannot create statistics on specified relation
DETAIL:  CREATE STATISTICS only supports tables, materialized views, foreign tables, and partitioned tables.

(Not sold on saying just "tables" vs. "regular tables" or "plain tables";
thoughts?)

While looking at this whole thing, I noticed that this code is somewhat
bogus in a different way: we're resolving the relation name twice, both
here in ProcessUtilitySlow() (to pass to transformStatsStmt), and later
again inside CreateStatistics().  It's really strange that we decided
not to pass the relation Oids as a list argument to CreateStatistics.  I
suggest to change that as the first attached patch.

Now, such a change would be appropriate only for branch master; it seems too
invasive for stable ones.  For them I propose we only change the error message,
as in the other attachment.

We should add a couple of test cases for this particular error message
also.  It's bad that these changes don't break any tests.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Once again, thank you and all of the developers for your hard work on
PostgreSQL.  This is by far the most pleasant management experience of
any database I've worked on."                             (Dan Harris)
http://archives.postgresql.org/pgsql-performance/2006-04/msg00247.php

Attachment

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: index prefetching
Next
From: Nathan Bossart
Date:
Subject: make LWLockCounter a global variable