make all ereport in gram.y print out relative location

Started by jian heover 1 year ago5 messageshackers
Jump to latest
#1jian he
jian.universality@gmail.com

hi.
it would be better to make all ereport in gram.y print out the
relative location.
one benefit is quickly locating the error.

in insertSelectOptions, some ereport won't call parser_errposition.
To handle that
I added a location to struct SelectLimit, so we can catch the location
in the LIMIT/FETCH/OFFSET clause.

in insertSelectOptions the error is
errmsg("WITH TIES cannot be specified without
ORDER BY clause"),
errmsg("%s and %s options cannot be used
together","SKIP LOCKED", "WITH TIES"),

to capture the exact location, i use the following way:
                        | FETCH first_or_next select_fetch_first_value
row_or_rows WITH TIES
@@ -13222,6 +13247,7 @@ limit_clause:
                                        n->limitOffset = NULL;
                                        n->limitCount = $3;
                                        n->limitOption = LIMIT_OPTION_WITH_TIES;
+                                       n->location = @5;
                                        $$ = n;

but normally we do `n->location = @1;`
not sure my change in insertSelectOptions is correct.

For other places in gram.y, I guess my change is sane.
i manually checked, after my patch, all ereport function calls in
gram.y will print out the relative location.

Attachments:

v1-0001-add-cursor-position-in-various-ereport-function-i.patchapplication/x-patch; name=v1-0001-add-cursor-position-in-various-ereport-function-i.patchDownload+86-39
#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: jian he (#1)
Re: make all ereport in gram.y print out relative location

Why remove parsePartitionStrategy? You could just add an "int location"
parameter to it.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Los cuentos de hadas no dan al niño su primera idea sobre los monstruos.
Lo que le dan es su primera idea de la posible derrota del monstruo."
(G. K. Chesterton)

#3jian he
jian.universality@gmail.com
In reply to: Alvaro Herrera (#2)
Re: make all ereport in gram.y print out relative location

On Tue, Oct 29, 2024 at 10:49 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

Why remove parsePartitionStrategy? You could just add an "int location"
parameter to it.

now changed to
static PartitionStrategy parsePartitionStrategy(char *strategy, int location,
core_yyscan_t yyscanner);

Attachments:

v2-0001-add-cursor-position-in-various-ereport-function-i.patchtext/x-patch; charset=US-ASCII; name=v2-0001-add-cursor-position-in-various-ereport-function-i.patchDownload+81-20
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: jian he (#3)
Re: make all ereport in gram.y print out relative location

jian he <jian.universality@gmail.com> writes:

now changed to
static PartitionStrategy parsePartitionStrategy(char *strategy, int location,
core_yyscan_t yyscanner);

I can take a look at this, since it's basically a followup
to 774171c4f.

regards, tom lane

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#4)
Re: make all ereport in gram.y print out relative location

I wrote:

I can take a look at this, since it's basically a followup
to 774171c4f.

Pushed with some cleanup and further hacking.

regards, tom lane