Обсуждение: transformJsonFuncExpr pathspec cache lookup failed

Поиск
Список
Период
Сортировка

transformJsonFuncExpr pathspec cache lookup failed

От
jian he
Дата:
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/

Вложения

Re: transformJsonFuncExpr pathspec cache lookup failed

От
Kirill Reshke
Дата:
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