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 202510201522.r7i6dr2wm2h7@alvherre.pgsql
Whole thread Raw
In response to Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt  (jian he <jian.universality@gmail.com>)
List pgsql-hackers
On 2025-Oct-20, jian he wrote:

> @@ -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.

Interesting, thanks for the example.

I think this example illustrates very well why the 'cmd' is rather
useless here -- note how the error message refers to an operator that
appears nowhere in the query string.  The user says "SET DATA TYPE text"
and then we complain about the "text+integer" operator?  That makes no
sense and I don't think it's in any way useful.

I think a user-friendly thing we could do is add an errcontext callback
that shows that we're trying to reapply a statistics object.  We should
discuss that in a separate thread for a separate patch though, and also
I don't think that thread should prevent this rework from being applied
now.  We could add this test scenario to the regression tests though, if
only to show how it would change later.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Should we say "wal_level = logical" instead of "wal_level >= logical"
Next
From: "David E. Wheeler"
Date:
Subject: Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()