Re: Cleaning up array_in()

From: Alexander Lakhin <exclusion(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Nikhil Benesch <nikhil(dot)benesch(at)gmail(dot)com>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Cleaning up array_in()
Date: 2023-09-13 06:00:00
Message-ID: 0c8e9673-42a5-5f12-6f3d-3a26449fd56b@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

12.09.2023 11:45, jian he wrote:
> On Mon, Sep 11, 2023 at 8:00 PM Alexander Lakhin <exclusion(at)gmail(dot)com> wrote:
>> I can confirm that all those anomalies are fixed now.
>> But new version brings a warning when compiled with gcc:
>> arrayfuncs.c:659:9: warning: variable 'prev_tok' is uninitialized when used here [-Wuninitialized]
>> if (prev_tok == ATOK_DELIM || nest_level == 0)
>> ^~~~~~~~
>> arrayfuncs.c:628:3: note: variable 'prev_tok' is declared here
>> ArrayToken prev_tok;
>> ^
>> 1 warning generated.
>>
>> Also it looks like an updated comment needs fixing/improving:
>> /* No array dimensions, so first literal character should be oepn curl-braces */
>> (should be an opening brace?)
>>
> fixed these 2 issues.
> --query
> SELECT ('{ ' || string_agg(chr((ascii('B') + round(random() * 25)) ::
> integer),', ') || ' }')::text[]
> FROM generate_series(1,1e6) \watch i=0.1 c=1
>
> After applying the patch, the above query runs slightly faster.

Thank you, Jian He!

Now I see only a few wrinkles.
1) A minor asymmetry in providing details appeared:
select E'{"a"a}'::text[];
ERROR:  malformed array literal: "{"a"a}"
LINE 1: select E'{"a"a}'::text[];
               ^
DETAIL:  Unexpected array element.

select E'{a"a"}'::text[];
ERROR:  malformed array literal: "{a"a"}"
LINE 1: select E'{a"a"}'::text[];
               ^
(no DETAIL)

Old behavior:
select E'{a"a"}'::text[];
ERROR:  malformed array literal: "{a"a"}"
LINE 1: select E'{a"a"}'::text[];
               ^
DETAIL:  Unexpected array element.

select E'{"a"a}'::text[];
ERROR:  malformed array literal: "{"a"a}"
LINE 1: select E'{"a"a}'::text[];
               ^
DETAIL:  Unexpected array element.

2) CPPFLAGS="-DARRAYDEBUG" ./configure ... breaks "make check", maybe change elog(NOTICE) to elog(DEBUG1)?
2a) a message logged there lacks some delimiter before "lBound info":
NOTICE:  array_in- ndim 1 ( 3 -1 -1 -1 -1 -1lBound info 1 1 1 1 1 1) for {red,green,blue}
what about changing the format to "ndim 1 ( 3 -1 -1 -1 -1 -1; lBound info: 1 1 1 1 1 1)"?

3) It seems that new comments need polishing, in particular:
 /* initialize dim, lBound. useful for ReadArrayDimensions ReadArrayStr */
->?
 /* Initialize dim, lBound for ReadArrayDimensions, ReadArrayStr */

Otherwise, we determine the dimensions from the in curly-braces
->?
Otherwise, we determine the dimensions from the curly braces.

Best regards,
Alexander

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message ywgrit 2023-09-13 06:00:49 Re: Is the member name of hashctl inappropriate?
Previous Message Michael Paquier 2023-09-13 05:27:37 Re: persist logical slots to disk during shutdown checkpoint