On Tue, Jan 28, 2025 at 11:25 AM jian he <[email protected]>
wrote:
> 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.
>
Noted.
> 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.
>
What error message do you think is appropriate in that situation?
> select '{}'::pg_ndistinct ;
> segfault
>
Mmm, gotta look into that!
>
>
> 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".
>
That sounds better.
> typcategory (Z) marked as Internal-use types. and there is no
> pg_ndistinct array type,
> not sure this is fine.
>
I think it's probably ok for now. The datatype currently has no utility
other than extended statistics, and I'm doubtful that it ever will.
