From 7e70e0643aebdb1b34788bb5cd8d2829bd0ff393 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 9d4cca0db2..68a7201e35 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -4303,11 +4303,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 5485104b6b..a2566d9a20 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 eeeb411e47..469ca542ad 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: could not coerce ON ERROR expression (FALSE) to the RETURNING type DETAIL: invalid input syntax for type smallint: "false" 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