From 32c2510695d36a0363a51056f05787744fe89cd7 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Mon, 22 Jul 2024 17:13:58 +0900 Subject: [PATCH 2/2] SQL/JSON: Fix casting for integer EXISTS columns in JSON_TABLE The current method of coercing the boolean result value of JsonPathExists() to the target type specified for an EXISTS column, which is to call the type's input function via json_populate_type(), leads to an error when the target type is integer, because the integer input function doesn't recognize boolean literal values as valid. Instead use the boolean-to-integer cast function for coercion in that case so that using integer as type for EXISTS columns works. Note that coercion for ON ERROR values TRUE and FALSE already works like that because the parser creates a cast expression including the cast function, but the coercion of the actual result value is not handled by the parser. Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com --- src/backend/executor/execExprInterp.c | 19 +++++++++++++++++++ src/backend/parser/parse_expr.c | 2 +- .../regress/expected/sqljson_jsontable.out | 10 ++++++++-- src/test/regress/sql/sqljson_jsontable.sql | 3 ++- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index d8735286c4..271a4b8f15 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -4304,11 +4304,30 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op, if (!error) { *op->resnull = false; + + /* + * Prepare to call json_populate_type() to coerce the + * boolean result to the target type. Parser must have set + * use_json_coercion to false if the target type is + * integer, because in that case we call the boolean-to- + * integer cast function instead as the integer's input + * function (which is what json_populate_type() for scalar + * target types) doesn't accept boolean literals as valid + * input. We only have a special case for integer as it + * seems common to use integer type for EXISTS columns in + * JSON_TABLE(). + */ if (jsexpr->use_json_coercion) + { + Assert(jsexpr->returning->typid != INT4OID); *op->resvalue = DirectFunctionCall1(jsonb_in, BoolGetDatum(exists) ? CStringGetDatum("true") : CStringGetDatum("false")); + } + else if (jsexpr->returning->typid == INT4OID) + *op->resvalue = DirectFunctionCall1(bool_int4, + BoolGetDatum(exists)); else *op->resvalue = BoolGetDatum(exists); } diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index d31b4ffcec..61611b8a59 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -4476,7 +4476,7 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) /* JSON_TABLE() COLUMNS can specify a non-boolean type. */ if (jsexpr->returning->typid != BOOLOID) - jsexpr->use_json_coercion = true; + jsexpr->use_json_coercion = jsexpr->returning->typid != INT4OID; jsexpr->on_error = transformJsonBehavior(pstate, func->on_error, JSON_BEHAVIOR_FALSE, diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out index 939e1b2f87..9e93307f85 100644 --- a/src/test/regress/expected/sqljson_jsontable.out +++ b/src/test/regress/expected/sqljson_jsontable.out @@ -175,7 +175,7 @@ FROM json_table_test vals [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 6 | f | 0 | | false [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 7 | f | 0 | | false [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 8 | f | 0 | | false - [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 9 | t | 0 | | true + [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 9 | t | 1 | 1 | true [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 10 | f | 0 | | false [1, 1.23, "2", "aaaaaaa", "foo", null, false, true, {"aaa": 123}, "[1,2]", "\"str\""] | 11 | f | 0 | | false (14 rows) @@ -548,12 +548,18 @@ SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int PATH 'lax $.a' DEFAULT (1 row) -- JSON_TABLE: EXISTS PATH types -SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int4 EXISTS PATH '$.a')); +SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int4 EXISTS PATH '$.a' ERROR ON ERROR)); -- ok; can cast to int4 a --- 0 (1 row) +SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int4 EXISTS PATH '$' ERROR ON ERROR)); -- ok; can cast to int4 + a +--- + 1 +(1 row) + SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int2 EXISTS PATH '$.a')); ERROR: cannot cast behavior expression of type boolean to smallint SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int8 EXISTS PATH '$.a')); diff --git a/src/test/regress/sql/sqljson_jsontable.sql b/src/test/regress/sql/sqljson_jsontable.sql index dc509bed8a..f6fc09b7ad 100644 --- a/src/test/regress/sql/sqljson_jsontable.sql +++ b/src/test/regress/sql/sqljson_jsontable.sql @@ -262,7 +262,8 @@ SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int PATH 'strict $.a' DEFAU SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int PATH 'lax $.a' DEFAULT 1 ON EMPTY DEFAULT 2 ON ERROR)) jt; -- JSON_TABLE: EXISTS PATH types -SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int4 EXISTS PATH '$.a')); +SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int4 EXISTS PATH '$.a' ERROR ON ERROR)); -- ok; can cast to int4 +SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int4 EXISTS PATH '$' ERROR ON ERROR)); -- ok; can cast to int4 SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int2 EXISTS PATH '$.a')); SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a int8 EXISTS PATH '$.a')); SELECT * FROM JSON_TABLE(jsonb '"a"', '$' COLUMNS (a float4 EXISTS PATH '$.a')); -- 2.43.0