Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt

jian he <jian.universality@gmail.com>

From: jian he <jian.universality@gmail.com>
To: Álvaro Herrera <alvherre@kurilemu.de>
Cc: Peter Eisentraut <peter@eisentraut.org>, Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Kirill Reshke <reshkekirill@gmail.com>
Date: 2025-10-20T12:03:06Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. CREATE STATISTICS: improve misleading error message

  2. Glossary: improve definition of "relation"

  3. Extended statistics on expressions

On Mon, Oct 20, 2025 at 3:31 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Oct-20, jian he wrote:
>
> > On Fri, Oct 17, 2025 at 8:13 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> > /*
> >  * transformStatsStmt - parse analysis for CREATE STATISTICS
> >  *
> >  * To avoid race conditions, it's important that this function relies only on
> >  * the passed-in rel (and not on stmt->relation) as the target relation.
> >  */
> > CreateStatsStmt *
> > transformStatsStmt(Relation rel, CreateStatsStmt *stmt, const char *queryString)
>
> > the (Relation rel) effectively comes from "stmt->relations", which
> > conflict with the above comments.
>
> Hmm?  It does not.  The whole point is that the relation name (RangeVar
> stmt->relations) has already been resolved to an OID and locked, which
> is what we pass as 'Relation rel'.  Trying to resolve the name to an OID
> again opens us up for race conditions.  This is alleviated if the
> relation has already been locked, so maybe we can relax this comment;
> but it's not outright contradictory AFAICS.
>

I think I understand your point.
When ALTER TABLE SET DATA TYPE invokes CreateStatistics, if the Relation (rel)
returned by CreateStatistics->relation_openrv is not the same as
ATPostAlterTypeParse.oldRelId, the regression test would already fail.


@@ -15632,11 +15623,6 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId,
Oid refRelId, char *cmd,
  querytree_list = lappend(querytree_list, stmt);
  querytree_list = list_concat(querytree_list, afterStmts);
  }
- else if (IsA(stmt, CreateStatsStmt))
- querytree_list = lappend(querytree_list,
- transformStatsStmt(oldRelId,
- (CreateStatsStmt *) stmt,
- cmd));

The above "cmd" is the queryString, which is useful for error reporting.

create table t(a  int);
create statistics xxx on (a + 1 is not null) from t;
alter table t alter column a set data type text;

with patch, v4-0001-Restructure-CreateStatsStmt-parse-analysis-proces.patch
ERROR:  operator does not exist: text + integer
DETAIL:  No operator of that name accepts the given argument types.
HINT:  You might need to add explicit type casts.


In the master branch, the error message also includes the error position.
ERROR:  operator does not exist: text + integer
LINE 1: alter table t alter column a set data type text;
                                            ^
DETAIL:  No operator of that name accepts the given argument types.
HINT:  You might need to add explicit type casts.