SQL/JSON: Improve error-handling of JsonBehavior expressions

Instead of returning a NULL when the JsonBehavior expression value
could not be coerced to the RETURNING type, throw the error message
informing the user that it is the JsonBehavior expression that caused
the error with the actual coercion error message shown in its DETAIL
line.

Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 4c9b2a8c17828e7e914eaec1e42eb92f164ebbc9..430438f668e4ab6061d724339c1ae71168b68e84 100644 (file)
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -4284,13 +4284,12 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
    memset(&jsestate->error, 0, sizeof(NullableDatum));
    memset(&jsestate->empty, 0, sizeof(NullableDatum));
 
-   /*
-    * Also reset ErrorSaveContext contents for the next row.  Since we don't
-    * set details_wanted, we don't need to also reset error_data, which would
-    * be NULL anyway.
-    */
-   Assert(!jsestate->escontext.details_wanted &&
-          jsestate->escontext.error_data == NULL);
+   /* Also reset ErrorSaveContext contents for the next row. */
+   if (jsestate->escontext.details_wanted)
+   {
+       jsestate->escontext.error_data = NULL;
+       jsestate->escontext.details_wanted = false;
+   }
    jsestate->escontext.error_occurred = false;
 
    switch (jsexpr->op)
@@ -4400,6 +4399,14 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
            error = true;
    }
 
+   /*
+    * When setting up the ErrorSaveContext (if needed) for capturing the
+    * errors that occur when coercing the JsonBehavior expression, set
+    * details_wanted to be able to show the actual error message as the
+    * DETAIL of the error message that tells that it is the JsonBehavior
+    * expression that caused the error; see ExecEvalJsonCoercionFinish().
+    */
+
    /* Handle ON EMPTY. */
    if (empty)
    {
@@ -4410,6 +4417,9 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
            if (jsexpr->on_empty->btype != JSON_BEHAVIOR_ERROR)
            {
                jsestate->empty.value = BoolGetDatum(true);
+               /* Set up to catch coercion errors of the ON EMPTY value. */
+               jsestate->escontext.error_occurred = false;
+               jsestate->escontext.details_wanted = true;
                Assert(jsestate->jump_empty >= 0);
                return jsestate->jump_empty;
            }
@@ -4417,6 +4427,9 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
        else if (jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR)
        {
            jsestate->error.value = BoolGetDatum(true);
+           /* Set up to catch coercion errors of the ON ERROR value. */
+           jsestate->escontext.error_occurred = false;
+           jsestate->escontext.details_wanted = true;
            Assert(!throw_error && jsestate->jump_error >= 0);
            return jsestate->jump_error;
        }
@@ -4442,6 +4455,9 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
        *op->resvalue = (Datum) 0;
        *op->resnull = true;
        jsestate->error.value = BoolGetDatum(true);
+       /* Set up to catch coercion errors of the ON ERROR value. */
+       jsestate->escontext.error_occurred = false;
+       jsestate->escontext.details_wanted = true;
        return jsestate->jump_error;
    }
 
@@ -4544,9 +4560,33 @@ ExecEvalJsonCoercion(ExprState *state, ExprEvalStep *op,
                                       (Node *) escontext);
 }
 
+static char *
+GetJsonBehaviorValueString(JsonBehavior *behavior)
+{
+   /*
+    * The order of array elements must correspond to the order of
+    * JsonBehaviorType members.
+    */
+   const char *behavior_names[] =
+   {
+       "NULL",
+       "ERROR",
+       "EMPTY",
+       "TRUE",
+       "FALSE",
+       "UNKNOWN",
+       "EMPTY ARRAY",
+       "EMPTY OBJECT",
+       "DEFAULT"
+   };
+
+   return pstrdup(behavior_names[behavior->btype]);
+}
+
 /*
  * Checks if an error occurred in ExecEvalJsonCoercion().  If so, this sets
- * JsonExprState.error to trigger the ON ERROR handling steps.
+ * JsonExprState.error to trigger the ON ERROR handling steps, unless the
+ * error is thrown when coercing a JsonBehavior value.
  */
 void
 ExecEvalJsonCoercionFinish(ExprState *state, ExprEvalStep *op)
@@ -4555,8 +4595,28 @@ ExecEvalJsonCoercionFinish(ExprState *state, ExprEvalStep *op)
 
    if (SOFT_ERROR_OCCURRED(&jsestate->escontext))
    {
+       /*
+        * jsestate->error or jsetate->empty being set means that the error
+        * occurred when coercing the JsonBehavior value.  Throw the error in
+        * that case with the actual coercion error message shown in the
+        * DETAIL part.
+        */
+       if (DatumGetBool(jsestate->error.value))
+           ereport(ERROR,
+                   (errcode(ERRCODE_DATATYPE_MISMATCH),
+                    errmsg("could not coerce ON ERROR expression (%s) to the RETURNING type",
+                           GetJsonBehaviorValueString(jsestate->jsexpr->on_error)),
+                    errdetail("%s", jsestate->escontext.error_data->message)));
+       else if (DatumGetBool(jsestate->empty.value))
+           ereport(ERROR,
+                   (errcode(ERRCODE_DATATYPE_MISMATCH),
+                    errmsg("could not coerce ON EMPTY expression (%s) to the RETURNING type",
+                           GetJsonBehaviorValueString(jsestate->jsexpr->on_empty)),
+                    errdetail("%s", jsestate->escontext.error_data->message)));
+
        *op->resvalue = (Datum) 0;
        *op->resnull = true;
+
        jsestate->error.value = BoolGetDatum(true);
 
        /*
@@ -4564,6 +4624,8 @@ ExecEvalJsonCoercionFinish(ExprState *state, ExprEvalStep *op)
         * JsonBehavior expression.
         */
        jsestate->escontext.error_occurred = false;
+       jsestate->escontext.error_occurred = false;
+       jsestate->escontext.details_wanted = true;
    }
 }
 

diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index 19817b4be8cbb5348c62114abf3a128040d4af67..fcad9cc02891a48eb0d12286a6bc6d4035044eb1 100644 (file)
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -227,11 +227,8 @@ SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
 
 SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
     COLUMNS (js1 jsonb_test_domain PATH '$.a2' DEFAULT 'foo'::jsonb_test_domain ON EMPTY));
- js1 
------
- 
-(1 row)
-
+ERROR:  could not coerce ON EMPTY expression (DEFAULT) to the RETURNING type
+DETAIL:  value for domain jsonb_test_domain violates check constraint "jsonb_test_domain_check"
 SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
     COLUMNS (js1 jsonb_test_domain PATH '$.a2' DEFAULT 'foo1'::jsonb_test_domain ON EMPTY));
  js1  

diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index ec8caee91c7c3466f0e6d883326a639d9f92a852..ab045e13590764bc08f3a8e0bc8cc9b0f60f182c 100644 (file)
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -313,11 +313,8 @@ SELECT JSON_VALUE(jsonb '"2017-02-20"', '$' RETURNING date) + 9;
 -- Test NULL checks execution in domain types
 CREATE DOMAIN sqljsonb_int_not_null AS int NOT NULL;
 SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null);
- json_value 
-------------
-           
-(1 row)
-
+ERROR:  could not coerce ON ERROR expression (NULL) to the RETURNING type
+DETAIL:  domain sqljsonb_int_not_null does not allow null values
 SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null ERROR ON ERROR);
 ERROR:  domain sqljsonb_int_not_null does not allow null values
 SELECT JSON_VALUE(jsonb 'null', '$' RETURNING sqljsonb_int_not_null DEFAULT 2 ON EMPTY ERROR ON ERROR);
@@ -1035,11 +1032,8 @@ SELECT JSON_QUERY(jsonb '{"a": 1}', '$.a' RETURNING sqljsonb_int_not_null);
 (1 row)
 
 SELECT JSON_QUERY(jsonb '{"a": 1}', '$.b' RETURNING sqljsonb_int_not_null);
- json_query 
-------------
-           
-(1 row)
-
+ERROR:  could not coerce ON EMPTY expression (NULL) to the RETURNING type
+DETAIL:  domain sqljsonb_int_not_null does not allow null values
 SELECT JSON_QUERY(jsonb '{"a": 1}', '$.b' RETURNING sqljsonb_int_not_null ERROR ON EMPTY ERROR ON ERROR);
 ERROR:  no SQL/JSON item found for specified path
 -- Test timestamptz passing and output
@@ -1232,11 +1226,8 @@ DROP TABLE test_jsonb_mutability;
 DROP FUNCTION ret_setint;
 CREATE DOMAIN queryfuncs_test_domain AS text CHECK (value <> 'foo');
 SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING queryfuncs_test_domain DEFAULT 'foo'::queryfuncs_test_domain ON EMPTY);
- json_value 
-------------
- 
-(1 row)
-
+ERROR:  could not coerce ON EMPTY expression (DEFAULT) to the RETURNING type
+DETAIL:  value for domain queryfuncs_test_domain violates check constraint "queryfuncs_test_domain_check"
 SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING queryfuncs_test_domain DEFAULT 'foo1'::queryfuncs_test_domain ON EMPTY);
  json_value 
 ------------