MIME-Version: 1.0
References: <CADkLM=dpz3KFnqP-dgJ-zvRvtjsa8UZv8wDAQdqho=qN3kX0Zg@mail.gmail.com>
 <4630636f-7720-4ce4-ab1c-253b390e5174@vondra.me> <CADkLM=cWdTvyAEQcxw-6er3umo=R1ZKU_9jf2ZinNxRf9MbQZg@mail.gmail.com>
 <b4ba8ed9-f1b3-4b6b-8612-ff42d65c18c6@vondra.me> <CADkLM=dnknZtCgdp2CmRp8gjkau1axYQ7SBMgcj74KgvjQzOwQ@mail.gmail.com>
In-Reply-To: <CADkLM=dnknZtCgdp2CmRp8gjkau1axYQ7SBMgcj74KgvjQzOwQ@mail.gmail.com>
From: jian he <jian.universality@gmail.com>
Date: Wed, 29 Jan 2025 00:24:59 +0800
Message-ID: <CACJufxEZYqocFdgn-x-bJMRBSk_zkS=ziGGkaSumteiPDksnsg@mail.gmail.com>
Subject: Re: Extended Statistics set/restore/clear functions.
To: Corey Huinker <corey.huinker@gmail.com>
Cc: Tomas Vondra <tomas@vondra.me>, pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CACJufxEZYqocFdgn-x-bJMRBSk_zkS%3DziGGkaSumteiPDksnsg%40mail.gmail.com>
Precedence: bulk

hi.
I reviewed 0001 only.

in src/backend/statistics/mvdistinct.c

no need #include "nodes/pg_list.h" since
src/include/statistics/statistics.h sub level include "nodes/pg_list.h"

no need #include "utils/palloc.h"
sicne #include "postgres.h"
already included it.


 select '[{"6, -32768,,": -11}]'::pg_ndistinct;
ERROR:  malformed pg_ndistinct: "[{"6, -32768,,": -11}]"
LINE 1: select '[{"6, -32768,,": -11}]'::pg_ndistinct;
               ^
DETAIL:  All ndistinct count values are scalar doubles.
imho, this errdetail message is not good.


select '{}'::pg_ndistinct ;
segfault


select '{"1,":"1"}'::pg_ndistinct ;
ERROR:  malformed pg_ndistinct: "{"1,":"1"}"
LINE 1: select '{"1,":"1"}'::pg_ndistinct ;
               ^
DETAIL:  All ndistinct attnum lists must be a comma separated list of attnums.

imho, this errdetail message is not good. would be better saying that
"length of list of attnums must be larger than 1".


select  pt1.typnamespace, pt1.typarray, pt1.typcategory, pt1.typname
from    pg_type pt1
where pt1.typname ~*'distinct';

 typnamespace | typarray | typcategory |   typname
--------------+----------+-------------+--------------
           11 |        0 | Z           | pg_ndistinct

typcategory (Z) marked as Internal-use types. and there is no
pg_ndistinct array type,
not sure this is fine.


all the errcode one pair of the parenthesis is unnecessary.
for example
                (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                 errmsg("malformed pg_ndistinct: \"%s\"", parse->str),
                 errdetail("Must begin with \"{\"")));
can change to
                errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
                 errmsg("malformed pg_ndistinct: \"%s\"", parse->str),
                 errdetail("Must begin with \"{\""));

see https://www.postgresql.org/docs/current/error-message-reporting.html