From: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
---|---|
To: | jian he <jian(dot)universality(at)gmail(dot)com> |
Cc: | Kirill Reshke <reshkekirill(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Pass ParseState as down to utility functions. |
Date: | 2024-12-06 14:01:03 |
Message-ID: | 202412061401.bvyhx42mk7rp@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2024-Dec-06, jian he wrote:
> From 6bf657c3b62b7460b317c42ce2f4fa0988acf1a0 Mon Sep 17 00:00:00 2001
> From: jian he <jian(dot)universality(at)gmail(dot)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/
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2024-12-06 14:48:37 | Re: refactor AlterDomainAddConstraint (alter domain add constraint) |
Previous Message | Daniel Gustafsson | 2024-12-06 13:46:23 | Re: Proposal to add a new URL data type. |