| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Staroverov Ilja <i(dot)staroverov(at)ftdata(dot)ru> |
| Cc: | "pgsql-bugs(at)lists(dot)postgresql(dot)org" <pgsql-bugs(at)lists(dot)postgresql(dot)org>, 303677365(at)qq(dot)com |
| Subject: | Re: SQL/JSON PASSING fails for char(n), name and domains over text |
| Date: | 2026-11-11 06:06:09 |
| Message-ID: | CA+HiwqFc6ZHHu1RbMtdCpT2X8_LV3k8Phrd7vsALMs+TbwatSA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Mon, Nov 9, 2026 at 9:58 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Fri, Oct 16, 2026 at 7:02 PM Staroverov Ilja <i(dot)staroverov(at)ftdata(dot)ru> wrote:
> >
> > Hi,
> >
> > The PASSING clause of JSON_EXISTS(), JSON_QUERY(), JSON_VALUE() and
> > JSON_TABLE() fails at runtime for string types other than text and
> > varchar:
> >
> > CREATE TABLE t (code char(3));
> > INSERT INTO t VALUES ('abc');
> > SELECT JSON_EXISTS(jsonb '["abc"]', '$[*] ? (@ == $c)' PASSING code AS c)
> > FROM t;
> > ERROR: could not convert value of type character to jsonpath
> >
> > SELECT JSON_EXISTS(jsonb '["postgres"]', '$[*] ? (@ == $u)'
> > PASSING current_user AS u);
> > ERROR: could not convert value of type name to jsonpath
> >
> > CREATE DOMAIN dtext AS text;
> > SELECT JSON_VALUE(jsonb 'null', '$v' PASSING 'x'::dtext AS v);
> > ERROR: could not convert value of type dtext to jsonpath
> >
> > The same values work with text or varchar, and a domain over int works
> > too. Reproduced on 18.6 and on master (aa7b9cf13). REL_17_STABLE has
> > the same code.
> >
> > The cause is a mismatch between the parser and the executor. For
> > PASSING arguments, transformJsonValueExpr() passes the types handled by
> > JsonItemFromDatum() as they are, without converting them to jsonb, but
> > its default branch does the same for any other type of the string
> > category. JsonItemFromDatum() handles only TEXTOID and VARCHAROID among
> > the string types and errors out for the rest. Non-string types, like
> > the domain over int, are converted to jsonb in the parser, which is why
> > they work.
> >
> > The attached patch makes JsonItemFromDatum() convert values of the
> > other string types using their output function, the same way
> > to_jsonb() converts bpchar and name values. Some notes on the choice:
> >
> > - char(n) values keep their padding, so 'ab'::char(3) is passed as
> > "ab ". That's consistent with to_jsonb(), jsonb_build_object(),
> > JSON_OBJECT(), JSON_ARRAY(), JSON_SCALAR() and with the vars argument
> > of jsonb_path_query(), which all give "ab ".
> >
> > - The parse tree doesn't change, so views and rules that use such
> > PASSING arguments start working too, and nothing changes for the
> > types that already work.
> >
> > - The cost is two syscache lookups and an output function call each
> > time the variable is read, so for these types a filter like
> > '$[*] ? (@ == $c)' over a large array is slower than with text (on a
> > debug build, about 2x for 200k elements). The alternative would be to
> > coerce the argument once per row when setting up the PASSING
> > variables, but that has to be done both in ExecInitJsonExpr() and in
> > the JSON_TABLE code, and it seemed too much for these types.
> >
> > The patch adds regression tests, which fail without the fix. It
> > applies to REL_18_STABLE as is. For REL_17_STABLE, the hunk adding
> > #include "utils/lsyscache.h" isn't needed, because the file already
> > includes it there.
>
> Thanks for the patch. I agree with fixing JsonItemFromDatum() to
> handle the types that the parser passes through to it.
>
> For bpchar and name, it seems better to add explicit cases to the
> switch, so that the cost you mention is only paid for domains over
> string types and for extension types. Attached is an updated version
> with that change, plus a test for the output-function path using a
> user-defined type. Also attached is the variant for 17 with the
> #include removed because it's already present.
>
> Also noticed that [1] reports this issue too among others, so I added
> that thread and its reporter (cc’d) to the commit message.
>
> I’ll push to master and back-patch to 17 later this week.
Done.
--
Thanks, Amit Langote
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-11-11 06:34:57 | Re: BUG #19842: pg_restore_extended_stats() rejects a faithful pg_dump of ANALYZE-produced range expression statisti |
| Previous Message | PG Bug reporting form | 2026-11-11 05:48:24 | BUG #19868: pg_resetwal prints uint32 WAL segment size as -1 bytes |