Re: Cleaning up array_in()

Поиск
Список
Период
Сортировка
От Nikhil Benesch
Тема Re: Cleaning up array_in()
Дата
Msg-id CAPWqQZQf3eV1UKw0x-uSML3yuvP_MYzvsxj_pYRy2+07fJ1soQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Cleaning up array_in()  (Alexander Lakhin <exclusion@gmail.com>)
Ответы Re: Cleaning up array_in()  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Cleaning up array_in()  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
I took a look at 0002 because I attempted a similar but more surgical
fix in [0].

I spotted a few opportunities for further reducing state tracked by
`ArrayCount`. You may not find all of these suggestions to be
worthwhile.

1) `in_quotes` appears to be wholly redundant with `parse_state ==
ARRAY_QUOTED_ELEM_STARTED`.

2) The `empty_array` special case does not seem to be important to
ArrayCount's callers, which don't even special case `ndims == 0` but
rather `ArrayGetNItemsSafe(..) == 0`. Perhaps this is a philosophical
question as to whether `ArrayCount('{{}, {}}')` should return
(ndims=2, dims=[2, 0]) or (ndims=0). Obviously someone needs to do
that normalization, but `ArrayCount` could leave that normalization to
`ReadArrayStr`.

3) `eoArray` could be replaced with a new `ArrayParseState` of
`ARRAY_END`. Just a matter of taste, but "end of array" feels like a
parser state to me.

I also have a sense that `ndims_frozen` made the distinction between
`ARRAY_ELEM_DELIMITED` and `ARRAY_LEVEL_DELIMITED` unnecessary, and
the two states could be merged into a single `ARRAY_DELIMITED` state,
but I've not pulled on this thread hard enough to say so confidently.

Thanks for doing the serious overhaul. As you say, the element
counting logic is much easier to follow now. I'm much more confident
that your patch is correct than mine.

Cheers,
Nikhil

[0]: https://www.postgresql.org/message-id/CAPWqQZRHsFuvWJj%3DczXuKEB03LF4ctPpDE1k3CoexweEFicBKQ%40mail.gmail.com


On Tue, May 9, 2023 at 7:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
>
> 09.05.2023 06:06, Tom Lane wrote:
> > Alexander Lakhin <exclusion@gmail.com> writes:
> >> The only thing that confused me, is the error message (it's not new, too):
> >> select '{{{{{{{{{{1}}}}}}}}}}'::int[];
> >> or even:
> >> select '{{{{{{{{{{'::int[];
> >> ERROR:  number of array dimensions (7) exceeds the maximum allowed (6)
> > Yeah, I didn't touch that, but it's pretty bogus because the first
> > number will always be "7" even if you wrote more than 7 left braces,
> > since the code errors out immediately upon finding that it's seen
> > too many braces.
> >
> > The equivalent message in the PLs just says "number of array dimensions
> > exceeds the maximum allowed (6)".  I'm inclined to do likewise in
> > array_in, but didn't touch it here.
>
> I think that, strictly speaking, we have no array dimensions in the string
> '{{{{{{{{{{'; there are only characters (braces) during the string parsing.
> While in the PLs we definitely deal with real arrays, which have dimensions.
>
> >> Beside that, I would like to note the following error text changes
> >> (all of these are feasible, I think):
> > I'll look into whether we can improve those, unless you had a patch
> > in mind already?
>
> Those messages looked more or less correct to me, I just wanted to note how they are
> changing (and haven't highlighted messages, that are not), but if you see here room
> for improvement, please look into it (I have no good formulations yet).
>
> Best regards,
> Alexander
>
>



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Amit Kapila
Дата:
Сообщение: Re: Reload configuration more frequently in apply worker.
Следующее
От: Bharath Rupireddy
Дата:
Сообщение: Re: New Table Access Methods for Multi and Single Inserts