Re: Pass ParseState as down to utility functions. - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: Pass ParseState as down to utility functions.
Date
Msg-id 202412061401.bvyhx42mk7rp@alvherre.pgsql
Whole thread Raw
List pgsql-hackers
On 2024-Dec-06, jian he wrote:

> From 6bf657c3b62b7460b317c42ce2f4fa0988acf1a0 Mon Sep 17 00:00:00 2001
> From: jian he <jian.universality@gmail.com>
> Date: Fri, 6 Dec 2024 16:37:18 +0800
> Subject: [PATCH v6 1/1] print out error position for some DDL command
> 
> doing this by passing the source_string to the existing ParseState
> or by making a new ParseState passing source_string to it.
> 
> With this patch, the following functions will printout the error position for certain error cases.
> 
> ATExecAddOf
> DefineType
> ATPrepAlterColumnType
> ATExecAlterColumnType
> DefineDomain
> AlterType
> transformAlterTableStmt

I think it would make more sense to write the commit message in terms of
the DDL commands that now report error position, than the C functions.
Such a list of commands does not need to be exhaustive; a
representative-enough sample probably suffices.

> @@ -943,11 +942,13 @@ DefineDomain(CreateDomainStmt *stmt)

>                  if (constr->is_no_inherit)
>                      ereport(ERROR,
> -                            errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> -                            errmsg("not-null constraints for domains cannot be marked NO INHERIT"));
> +                            (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> +                             errmsg("not-null constraints for domains cannot be marked NO INHERIT"),
> +                             parser_errposition(pstate, constr->location)));

Once upon a time, ereport() was a simpler macro that did not
use variadic arguments.  Back then, the list of functions embedded in it
(errcode, errmsg etc) were forced to be in an additional level of
parentheses so that the macro would work at all (IIRC failure to do that
resulted in strange compile-time problems).  This is why a majority of
code is written in the style with those parens.  But commit e3a87b4991cc
changed ereport to use __VA_ARGS__, so the auxiliary functions are
actual arguments to errstart() -- which means that the parentheses
you're adding here are unnecessary and discouraged.  Just add the
parser_errposition() call and it'll be fine.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"XML!" Exclaimed C++.  "What are you doing here? You're not a programming
language."
"Tell that to the people who use me," said XML.
https://burningbird.net/the-parable-of-the-languages/



pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: Proposal to add a new URL data type.
Next
From: Alvaro Herrera
Date:
Subject: Re: refactor AlterDomainAddConstraint (alter domain add constraint)