From 4d7e24e0f4d05b546228488ccdc2848a80245ffb Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 6 Sep 2024 12:53:01 +0900 Subject: [PATCH] Revert recent SQL/JSON related commits Reverts 68222851d5a8, 565caaa79af, and 3a97460970f, because a few BF animals didn't like one or all of them. --- src/backend/executor/execExpr.c | 30 ++++--------- src/backend/parser/parse_expr.c | 4 +- src/backend/utils/adt/ruleutils.c | 10 ++--- .../regress/expected/sqljson_jsontable.out | 43 ------------------- src/test/regress/sql/sqljson_jsontable.sql | 10 ----- 5 files changed, 15 insertions(+), 82 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 4d247538bea..63289ee35ee 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -4414,8 +4414,6 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state, ErrorSaveContext *escontext = jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR ? &jsestate->escontext : NULL; - bool returning_domain = - get_typtype(jsexpr->returning->typid) == TYPTYPE_DOMAIN; jsestate->jsexpr = jsexpr; @@ -4558,27 +4556,20 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state, ExprEvalPushStep(state, scratch); } + jsestate->jump_empty = jsestate->jump_error = -1; + /* * Step to check jsestate->error and return the ON ERROR expression if * there is one. This handles both the errors that occur during jsonpath * evaluation in EEOP_JSONEXPR_PATH and subsequent coercion evaluation. - * - * Speed up common cases by avoiding extra steps for a NULL-valued ON - * ERROR expression unless RETURNING a domain type, where constraints must - * be checked. ExecEvalJsonExprPath() already returns NULL on error, - * making additional steps unnecessary in typical scenarios. Note that the - * default ON ERROR behavior for JSON_VALUE() and JSON_QUERY() is to - * return NULL. */ - jsestate->jump_error = state->steps_len; if (jsexpr->on_error && - jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR && - (!(IsA(jsexpr->on_error->expr, Const) && - ((Const *) jsexpr->on_error->expr)->constisnull) || - returning_domain)) + jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR) { ErrorSaveContext *saved_escontext; + jsestate->jump_error = state->steps_len; + /* JUMP to end if false, that is, skip the ON ERROR expression. */ jumps_to_end = lappend_int(jumps_to_end, state->steps_len); scratch->opcode = EEOP_JUMP_IF_NOT_TRUE; @@ -4628,19 +4619,14 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state, /* * Step to check jsestate->empty and return the ON EMPTY expression if * there is one. - * - * See the comment above for details on the optimization for NULL-valued - * expressions. */ - jsestate->jump_empty = state->steps_len; if (jsexpr->on_empty != NULL && - jsexpr->on_empty->btype != JSON_BEHAVIOR_ERROR && - (!(IsA(jsexpr->on_empty->expr, Const) && - ((Const *) jsexpr->on_empty->expr)->constisnull) || - returning_domain)) + jsexpr->on_empty->btype != JSON_BEHAVIOR_ERROR) { ErrorSaveContext *saved_escontext; + jsestate->jump_empty = state->steps_len; + /* JUMP to end if false, that is, skip the ON EMPTY expression. */ jumps_to_end = lappend_int(jumps_to_end, state->steps_len); scratch->opcode = EEOP_JUMP_IF_NOT_TRUE; diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 36c1b7a88f2..56e413da9f5 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -4603,13 +4603,13 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) } /* - * Assume EMPTY ARRAY ON ERROR when ON ERROR is not specified. + * Assume EMPTY ON ERROR when ON ERROR is not specified. * * ON EMPTY cannot be specified at the top level but it can be for * the individual columns. */ jsexpr->on_error = transformJsonBehavior(pstate, func->on_error, - JSON_BEHAVIOR_EMPTY_ARRAY, + JSON_BEHAVIOR_EMPTY, jsexpr->returning); break; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index cd9c3eddd1d..b31be31321d 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -11719,6 +11719,7 @@ get_json_table_columns(TableFunc *tf, JsonTablePathScan *scan, bool showimplicit) { StringInfo buf = context->buf; + JsonExpr *jexpr = castNode(JsonExpr, tf->docexpr); ListCell *lc_colname; ListCell *lc_coltype; ListCell *lc_coltypmod; @@ -11771,10 +11772,6 @@ get_json_table_columns(TableFunc *tf, JsonTablePathScan *scan, if (ordinality) continue; - /* - * Set default_behavior to guide get_json_expr_options() on whether to - * to emit the ON ERROR / EMPTY clauses. - */ if (colexpr->op == JSON_EXISTS_OP) { appendStringInfoString(buf, " EXISTS"); @@ -11798,6 +11795,9 @@ get_json_table_columns(TableFunc *tf, JsonTablePathScan *scan, default_behavior = JSON_BEHAVIOR_NULL; } + if (jexpr->on_error->btype == JSON_BEHAVIOR_ERROR) + default_behavior = JSON_BEHAVIOR_ERROR; + appendStringInfoString(buf, " PATH "); get_json_path_spec(colexpr->path_spec, context, showimplicit); @@ -11875,7 +11875,7 @@ get_json_table(TableFunc *tf, deparse_context *context, bool showimplicit) get_json_table_columns(tf, castNode(JsonTablePathScan, tf->plan), context, showimplicit); - if (jexpr->on_error->btype != JSON_BEHAVIOR_EMPTY_ARRAY) + if (jexpr->on_error->btype != JSON_BEHAVIOR_EMPTY) get_json_behavior(jexpr->on_error, context, "ERROR"); if (PRETTY_INDENT(context)) diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out index 5c7aaa6159d..721e01d6ad0 100644 --- a/src/test/regress/expected/sqljson_jsontable.out +++ b/src/test/regress/expected/sqljson_jsontable.out @@ -1132,46 +1132,3 @@ ERROR: invalid ON ERROR behavior for column "a" LINE 1: ...M JSON_TABLE(jsonb '1', '$' COLUMNS (a int exists empty obje... ^ DETAIL: Only ERROR, TRUE, FALSE, or UNKNOWN is allowed in ON ERROR for EXISTS columns. --- Test JSON_TABLE() column deparsing -- don't emit default ON ERROR / EMPTY --- behavior -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$')); - QUERY PLAN ------------------------------------------------------------------------------------------------------ - Table Function Scan on "json_table" (cost=0.01..1.00 rows=100 width=32) - Output: a - Table Function Call: JSON_TABLE('"a"'::jsonb, '$' AS json_table_path_0 COLUMNS (a text PATH '$')) -(3 rows) - -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$') ERROR ON ERROR); - QUERY PLAN --------------------------------------------------------------------------------------------------------------------- - Table Function Scan on "json_table" (cost=0.01..1.00 rows=100 width=32) - Output: a - Table Function Call: JSON_TABLE('"a"'::jsonb, '$' AS json_table_path_0 COLUMNS (a text PATH '$') ERROR ON ERROR) -(3 rows) - --- Test JSON_TABLE() deparsing -- don't emit default ON ERROR behavior -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$')); - QUERY PLAN ------------------------------------------------------------------------------------------------------ - Table Function Scan on "json_table" (cost=0.01..1.00 rows=100 width=32) - Output: a - Table Function Call: JSON_TABLE('"a"'::jsonb, '$' AS json_table_path_0 COLUMNS (a text PATH '$')) -(3 rows) - -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$') EMPTY ON ERROR); - QUERY PLAN ------------------------------------------------------------------------------------------------------ - Table Function Scan on "json_table" (cost=0.01..1.00 rows=100 width=32) - Output: a - Table Function Call: JSON_TABLE('"a"'::jsonb, '$' AS json_table_path_0 COLUMNS (a text PATH '$')) -(3 rows) - -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$') EMPTY ARRAY ON ERROR); - QUERY PLAN ------------------------------------------------------------------------------------------------------ - Table Function Scan on "json_table" (cost=0.01..1.00 rows=100 width=32) - Output: a - Table Function Call: JSON_TABLE('"a"'::jsonb, '$' AS json_table_path_0 COLUMNS (a text PATH '$')) -(3 rows) - diff --git a/src/test/regress/sql/sqljson_jsontable.sql b/src/test/regress/sql/sqljson_jsontable.sql index 31bc9c9ea0c..38992316f5a 100644 --- a/src/test/regress/sql/sqljson_jsontable.sql +++ b/src/test/regress/sql/sqljson_jsontable.sql @@ -542,13 +542,3 @@ SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int) NULL ON ERROR); SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int true on empty)); SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int omit quotes true on error)); SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int exists empty object on error)); - --- Test JSON_TABLE() column deparsing -- don't emit default ON ERROR / EMPTY --- behavior -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$')); -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$') ERROR ON ERROR); - --- Test JSON_TABLE() deparsing -- don't emit default ON ERROR behavior -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$')); -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$') EMPTY ON ERROR); -EXPLAIN VERBOSE SELECT * from JSON_TABLE('"a"', '$' COLUMNS (a text PATH '$') EMPTY ARRAY ON ERROR); -- 2.30.2