Horton

Thread

Commits

  1. Add a parse location field to struct FunctionParameter.

  2. Make all ereport() calls within gram.y provide error locations.

  1. make all ereport in gram.y print out relative location

    jian he <jian.universality@gmail.com> — 2024-10-26T15:12:37Z

    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.

  2. Re: make all ereport in gram.y print out relative location

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2024-10-29T14:48:57Z

    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)

  3. Re: make all ereport in gram.y print out relative location

    jian he <jian.universality@gmail.com> — 2024-10-30T02:05:00Z

    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);

  4. Re: make all ereport in gram.y print out relative location

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-10-31T15:05:41Z

    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

  5. Re: make all ereport in gram.y print out relative location

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-10-31T20:11:30Z

    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