Re: transformJsonFuncExpr pathspec cache lookup failed - Mailing list pgsql-hackers

From Kirill Reshke
Subject Re: transformJsonFuncExpr pathspec cache lookup failed
Date
Msg-id CALdSSPiHtj4R3vuOuP7qMpO8XV4uYZ_=4j_Upcn0VR97GepboA@mail.gmail.com
Whole thread Raw
In response to transformJsonFuncExpr pathspec cache lookup failed  (jian he <jian.universality@gmail.com>)
List pgsql-hackers
On Fri, 7 Nov 2025 at 07:50, jian he <jian.universality@gmail.com> wrote:
>
> hi.
>
> in transformJsonFuncExpr:
>
>     path_spec = transformExprRecurse(pstate, func->pathspec);
>     path_spec = coerce_to_target_type(pstate, path_spec, exprType(path_spec),
>                                       JSONPATHOID, -1,
>                                       COERCION_EXPLICIT, COERCE_IMPLICIT_CAST,
>                                       exprLocation(path_spec));
>     if (path_spec == NULL)
>         ereport(ERROR,
>                 (errcode(ERRCODE_DATATYPE_MISMATCH),
>                  errmsg("JSON path expression must be of type %s, not
> of type %s",
>                         "jsonpath", format_type_be(exprType(path_spec))),
>                  parser_errposition(pstate, exprLocation(path_spec))));
>
> There is no test for this, if you try it, you can easily reach "cache
> lookup failed".
> SELECT JSON_VALUE(jsonb 'null', NULL::date);
> ERROR:  cache lookup failed for type 0
>
> because we first call ``format_type_be(exprType(path_spec))),`` then ereport.
> format_type_be can not code with InvalidOid.
>
> A patch is attached.
> -----------
> Also, note that we allow:
> SELECT * FROM JSON_TABLE(jsonb'"1.23"', '$' COLUMNS (js2 int PATH '$'));
> but don't allow
> SELECT * FROM JSON_TABLE(jsonb'"1.23"', '$'::jsonpath COLUMNS (js2 int
> PATH '$'));
>
> Maybe we should support this.
> since every A_Const should have a type for it. Allowing something like:
> JSON_TABLE(jsonb '"1.23"', '$'::some_jsonpath_type ... )
> seems consistent.
> I guess that's a separate issue, so I didn't touch it.
>
> --
> jian
> https://www.enterprisedb.com/

Hi!
I tried your fix and this indeed fixes an issue. Two minor comments:

First,
in the `src/backend/parser/parse_expr.c` fil there are multiple
examples of working with `coerce_to_target_type`, they all share
different coding practice:

```
coerced_expr = coerce_to_target_type(.., expr, ..)

if (coerced_expr == NULL)
     ereport();


expr = coerced_expr;
```

Instead of
```
expr = coerce_to_target_type(.., expr, ..)

if (expr == NULL)
     ereport();
```

Let's be consistent?


Second, about  allowing  JSON_TABLE(jsonb '"1.23"',
'$'::some_jsonpath_type ... ). The way we check input for JSON_TABLE
is we simply check for node type in the parser.
So, I think it may be too difficult to change that, because we do not
work with catalog in parser, so we cannot resolve if the type is
indeed some_json_type?

What we can do is we can change parser rules, to allow more
user-friendly error here:
```
reshke=# SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
    COLUMNS (js1 text PATH '$.d1'::jsonpath ));
ERROR:  syntax error at or near "::"
LINE 2:     COLUMNS (js1 text PATH '$.d1'::jsonpath ));
                                         ^
```

The way we can achieve it is to change the second token in
json_table_column_path_clause_opt to a_expr and give a more clear
error.


Thoughts?

-- 
Best regards,
Kirill Reshke



pgsql-hackers by date:

Previous
From: Bertrand Drouvot
Date:
Subject: Re: Consistently use the XLogRecPtrIsInvalid() macro
Next
From: Fujii Masao
Date:
Subject: Re: doc: Improve description of io_combine_limit and io_max_combine_limit GUCs