Re: Cleaning up array_in()

jian he <jian.universality@gmail.com>

From: jian he <jian.universality@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Nikhil Benesch <nikhil.benesch@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2023-07-03T04:16:22Z
Lists: pgsql-hackers
On Mon, Jun 5, 2023 at 9:48 AM Nikhil Benesch <nikhil.benesch@gmail.com> wrote:
>
> 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.

I pull ArrayCount into a separate C function, regress test (manually)
based on patch regress test.

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

removing it works as expected.

> 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.

works. (reduce one variable.)

> 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.

merging these states into one work as expected.



Commits

  1. Don't specify number of dimensions in cases where we don't know it.

  2. Improve readability and error detection of array_in().

  3. Add trailing commas to enum definitions