public inbox for [email protected]
help / color / mirror / Atom feedFrom: jian he <[email protected]>
To: Corey Huinker <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: Extended Statistics set/restore/clear functions.
Date: Thu, 13 Nov 2025 19:51:00 +0800
Message-ID: <CACJufxFni1oB-VFuLbVs9VSgVx0czvoZyoW0z_tJ2kq4E3SXYw@mail.gmail.com> (raw)
In-Reply-To: <CADkLM=fky861BrifcuWc0CbWfo3yMbRB+m1jG4qqXG_3hnqdtA@mail.gmail.com>
References: <CADkLM=efY2h8Bio=bE8O2Tnd=WUdyKDL1Zrtz9h+at16E4HVUA@mail.gmail.com>
<[email protected]>
<[email protected]>
<CADkLM=cTVN3g7OzdSXfnBHEYNVQiGMJ8cdx1v=zJa86w1A9ULA@mail.gmail.com>
<[email protected]>
<CADkLM=dMSJ5pHQoRxj9qoU49s+f33kXbhvm_12g=ARaXgysg=g@mail.gmail.com>
<[email protected]>
<CADkLM=dWQ3r48eAP8NggLqe90_16JKbit9iu9AtuUrZ8+A=qBA@mail.gmail.com>
<[email protected]>
<CADkLM=eGwDZX9DQ_GouzqjMG7x_kCedBS6br2SciJ3wjwivfZg@mail.gmail.com>
<[email protected]>
<CACJufxHZzUeu_fyem2oA_FSrjk0ky5vn_h1Dw68=UVLekXPjXA@mail.gmail.com>
<CACJufxHP+wZ9Th2U9JW27cCq-me222e76pv7vcjqt+oCL03m5Q@mail.gmail.com>
<CADkLM=c8-U4GLMw5VdeDdfp1ae6BW=PCfEQqAky04iZMbckCFw@mail.gmail.com>
<CACJufxFo5b0FvmV4CmOb74NFrXm_znZHh7bLiiVkAzVzTnJiVw@mail.gmail.com>
<CADkLM=fky861BrifcuWc0CbWfo3yMbRB+m1jG4qqXG_3hnqdtA@mail.gmail.com>
hi.
now looking at v12-0003-Add-working-input-function-for-pg_ndistinct.patch
again.
+ * example input:
+ * [{"attributes": [6, -1], "ndistinct": 14},
+ * {"attributes": [6, -2], "ndistinct": 9143},
+ * {"attributes": [-1,-2], "ndistinct": 13454},
+ * {"attributes": [6, -1, -2], "ndistinct": 14549}]
*/
Datum
pg_ndistinct_in(PG_FUNCTION_ARGS)
extenssted statistics surely won't work on system columns,
how should we deal with case like:
```
{"attributes": [6, -1], "ndistinct": 14}
{"attributes": [6, -7], "ndistinct": 14},
```
issue a warning or error out saying that your attribute number is invalid?
Should we discourage using system columns as examples in comments here?
I have added more test code in src/test/regress/sql/pg_ndistinct.sql,
to improve the code coverage.
as mentioned before in
https://postgr.es/m/CACJufxEZYqocFdgn-x-bJMRBSk_zkS=ziGGkaSumteiPDksnsg@mail.gmail.com
I think it's a good thing to change
``(errcode....``
to
``errcode``.
So I did the change.
+static JsonParseErrorType
+ndistinct_array_element_start(void *state, bool isnull)
+{
+ NDistinctParseState *parse = state;
+
+ switch(parse->state)
+ {
+ case NDIST_EXPECT_ATTNUM:
+ if (!isnull)
+ return JSON_SUCCESS;
+
+ ereturn(parse->escontext, (Datum) 0,
+ (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg("malformed pg_ndistinct: \"%s\"", parse->str),
+ errdetail("Attnum list elements cannot be null.")));
this (and many other places) looks wrong, because
ereturn would really return ``(Datum) 0``, and this function returns
JsonParseErrorType.
so we have to errsave here.
+typedef struct
+{
+ const char *str;
+ NDistinctSemanticState state;
+
+ List *distinct_items; /* Accumulated complete MVNDistinctItems */
+ Node *escontext;
+
+ bool found_attributes; /* Item has an attributes key */
+ bool found_ndistinct; /* Item has ndistinct key */
+ List *attnum_list; /* Accumulated attributes attnums */
+ int64 ndistinct;
+} NDistinctParseState;
+ case NDIST_EXPECT_NDISTINCT:
+ /*
+ * While the structure dictates that ndistinct in a double precision
+ * floating point, in practice it has always been an integer, and it
+ * is output as such. Therefore, we follow usage precendent over the
+ * actual storage structure, and read it in as an integer.
+ */
+ parse->ndistinct = pg_strtoint64_safe(token, parse->escontext);
+
+ if (SOFT_ERROR_OCCURRED(parse->escontext))
+ return JSON_SEM_ACTION_FAILED;
NDistinctParseState.ndistinct should be integer,
otherwise pg_ndistinct_out will not be consistent with pg_ndistinct_in?
SELECT '[{"attributes" : [1, 2], "ndistinct" :
2147483648}]'::pg_ndistinct; --error
pg_ndistinct
----------------------------------------------------
[{"attributes": [1, 2], "ndistinct": -2147483648}]
(1 row)
The result seems not what we expected.
Attachments:
[application/octet-stream] v13-0001-refactor-v12-0003.no-cfbot (17.9K, ../CACJufxFni1oB-VFuLbVs9VSgVx0czvoZyoW0z_tJ2kq4E3SXYw@mail.gmail.com/2-v13-0001-refactor-v12-0003.no-cfbot)
download
view thread (91+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Extended Statistics set/restore/clear functions.
In-Reply-To: <CACJufxFni1oB-VFuLbVs9VSgVx0czvoZyoW0z_tJ2kq4E3SXYw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox