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

From jian he
Subject Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt
Date
Msg-id CACJufxESPwhzVB5xQz6L1bhmjyUMwHrVt7w-NFRB0-vO+FAqzw@mail.gmail.com
Whole thread Raw
In response to Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt  (Álvaro Herrera <alvherre@kurilemu.de>)
Responses Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt
List pgsql-hackers
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.



pgsql-hackers by date:

Previous
From: "Hayato Kuroda (Fujitsu)"
Date:
Subject: RE: Logical Replication of sequences
Next
From: Tom Lane
Date:
Subject: Re: Question on ThrowErrorData