Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt
Álvaro Herrera <alvherre@kurilemu.de>
From: Álvaro Herrera <alvherre@kurilemu.de>
To: jian he <jian.universality@gmail.com>
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-17T12:13:08Z
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 →
-
CREATE STATISTICS: improve misleading error message
- cbd3732cbbaa 14.20 landed
- 4529861cc408 15.15 landed
- f30c04682cdf 16.11 landed
- f9b41f3e1ecd 17.7 landed
- 38883916e41c 18.0 landed
- 9ec0b29976b6 19 (unreleased) landed
- f9a7622483c3 15.15 landed
- f225473cbae2 19 (unreleased) landed
- e9fb314c7ad8 17.7 landed
- d84a6c3dad1f 16.11 landed
- b830f9d67463 14.20 landed
- 3eea4dc2c7cf 18.0 landed
-
Glossary: improve definition of "relation"
- 16a9165ce4a4 19 (unreleased) landed
-
Extended statistics on expressions
- a4d75c86bf15 14.0 cited
Attachments
On 2025-Oct-02, jian he wrote: > hi. > > moving all code in ProcessUtilitySlow (case T_CreateStatsStmt:) > to CreateStatistic seems more intuitive to me. > > so I rebased v3. Yeah, this looks good. But I think we can go a little further. Because CreateStatistics() now calls transformStatsStmt(), we don't need to do the same in ATPostAlterTypeParse(), which allows us to simplify the code there. In turn this means we can pass the whole Relation rather than merely the OID, so transformStatsStmt doesn't need to open it. I attach v4 with those changes. Comments? For a moment it looked weird to me to pass a Relation to the parse analysis routine, but I see that other functions declared in parse_utilcmd.h are already doing that. One more thing I noticed is that we're scribbling on the parser node, which we can easily avoid by creating a copy of the node in transformStatsStmt() before doing any changes. I'm not too clear on whether this is really necessary or not. We do it in other places, but we haven't done it here for a long while, and nothing seems to break because of that. diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 89c8315117b..7797c707f73 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -3150,6 +3150,9 @@ transformStatsStmt(Relation rel, CreateStatsStmt *stmt, const char *queryString) if (stmt->transformed) return stmt; + /* create a copy to scribble on */ + stmt = copyObject(stmt); + /* Set up pstate */ pstate = make_parsestate(NULL); pstate->p_sourcetext = queryString; -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "Uno puede defenderse de los ataques; contra los elogios se esta indefenso"