From 9dc7f323b7a1755f032da5627906c5808ee8271d Mon Sep 17 00:00:00 2001 From: jian he Date: Fri, 27 Jun 2025 09:33:53 +0800 Subject: [PATCH v11 1/1] refactoring v11_01 to v11_06 refactoring SubscriptingRef node refactoring SubscriptingRef deparsing other msic refactoring based on v11-0001 to v11-0006 --- src/backend/executor/execExpr.c | 24 ++---- src/backend/nodes/nodeFuncs.c | 73 +++--------------- src/backend/parser/parse_collate.c | 21 +----- src/backend/utils/adt/jsonbsubs.c | 111 ++++++++++++---------------- src/backend/utils/adt/ruleutils.c | 13 +++- src/test/regress/expected/jsonb.out | 24 +++++- src/test/regress/sql/jsonb.sql | 8 ++ 7 files changed, 107 insertions(+), 167 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index b0459011639..f1569879b52 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -3336,15 +3336,9 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, { sbsrefstate->upperprovided[i] = true; /* Each subscript is evaluated into appropriate array entry */ - if (IsA(e, String)) - { - sbsrefstate->upperindex[i] = CStringGetTextDatum(strVal(e)); - sbsrefstate->upperindexnull[i] = false; - } - else - ExecInitExprRec(e, state, - &sbsrefstate->upperindex[i], - &sbsrefstate->upperindexnull[i]); + ExecInitExprRec(e, state, + &sbsrefstate->upperindex[i], + &sbsrefstate->upperindexnull[i]); } i++; } @@ -3365,15 +3359,9 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, { sbsrefstate->lowerprovided[i] = true; /* Each subscript is evaluated into appropriate array entry */ - if (IsA(e, String)) - { - sbsrefstate->lowerindex[i] = CStringGetTextDatum(strVal(e)); - sbsrefstate->lowerindexnull[i] = false; - } - else - ExecInitExprRec(e, state, - &sbsrefstate->lowerindex[i], - &sbsrefstate->lowerindexnull[i]); + ExecInitExprRec(e, state, + &sbsrefstate->lowerindex[i], + &sbsrefstate->lowerindexnull[i]); } i++; } diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index a9c29ab8f29..e5390bd598c 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -2182,28 +2182,12 @@ expression_tree_walker_impl(Node *node, case T_SubscriptingRef: { SubscriptingRef *sbsref = (SubscriptingRef *) node; - ListCell *lc; - - /* - * Recurse directly for upper/lower container index lists, - * skipping String subscripts used for dot notation. - */ - foreach(lc, sbsref->refupperindexpr) - { - Node *expr = lfirst(lc); - - if (expr && !IsA(expr, String) && WALK(expr)) - return true; - } - - foreach(lc, sbsref->reflowerindexpr) - { - Node *expr = lfirst(lc); - - if (expr && !IsA(expr, String) && WALK(expr)) - return true; - } + /* recurse directly for upper/lower container index lists */ + if (LIST_WALK(sbsref->refupperindexpr)) + return true; + if (LIST_WALK(sbsref->reflowerindexpr)) + return true; /* walker must see the refexpr and refassgnexpr, however */ if (WALK(sbsref->refexpr)) return true; @@ -3098,51 +3082,12 @@ expression_tree_mutator_impl(Node *node, { SubscriptingRef *sbsref = (SubscriptingRef *) node; SubscriptingRef *newnode; - ListCell *lc; - List *exprs = NIL; FLATCOPY(newnode, sbsref, SubscriptingRef); - - foreach(lc, sbsref->refupperindexpr) - { - Node *expr = lfirst(lc); - - if (expr && IsA(expr, String)) - { - String *str; - - FLATCOPY(str, expr, String); - expr = (Node *) str; - } - else - expr = mutator(expr, context); - - exprs = lappend(exprs, expr); - } - - newnode->refupperindexpr = exprs; - - exprs = NIL; - - foreach(lc, sbsref->reflowerindexpr) - { - Node *expr = lfirst(lc); - - if (expr && IsA(expr, String)) - { - String *str; - - FLATCOPY(str, expr, String); - expr = (Node *) str; - } - else - expr = mutator(expr, context); - - exprs = lappend(exprs, expr); - } - - newnode->reflowerindexpr = exprs; - + MUTATE(newnode->refupperindexpr, sbsref->refupperindexpr, + List *); + MUTATE(newnode->reflowerindexpr, sbsref->reflowerindexpr, + List *); MUTATE(newnode->refexpr, sbsref->refexpr, Expr *); MUTATE(newnode->refassgnexpr, sbsref->refassgnexpr, diff --git a/src/backend/parser/parse_collate.c b/src/backend/parser/parse_collate.c index be6dea6ffd2..a74e63aa54c 100644 --- a/src/backend/parser/parse_collate.c +++ b/src/backend/parser/parse_collate.c @@ -680,24 +680,11 @@ assign_collations_walker(Node *node, assign_collations_context *context) * contribute anything.) */ SubscriptingRef *sbsref = (SubscriptingRef *) node; - ListCell *lc; - /* skip String subscripts used for dot notation */ - foreach(lc, sbsref->refupperindexpr) - { - Node *expr = lfirst(lc); - - if (expr && !IsA(expr, String)) - assign_expr_collations(context->pstate, expr); - } - - foreach(lc, sbsref->reflowerindexpr) - { - Node *expr = lfirst(lc); - - if (expr && !IsA(expr, String)) - assign_expr_collations(context->pstate, expr); - } + assign_expr_collations(context->pstate, + (Node *) sbsref->refupperindexpr); + assign_expr_collations(context->pstate, + (Node *) sbsref->reflowerindexpr); (void) assign_collations_walker((Node *) sbsref->refexpr, &loccontext); diff --git a/src/backend/utils/adt/jsonbsubs.c b/src/backend/utils/adt/jsonbsubs.c index 3588a1d062f..900af1c739b 100644 --- a/src/backend/utils/adt/jsonbsubs.c +++ b/src/backend/utils/adt/jsonbsubs.c @@ -14,6 +14,7 @@ */ #include "postgres.h" +#include "catalog/pg_collation_d.h" #include "executor/execExpr.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" @@ -25,6 +26,7 @@ #include "utils/jsonpath.h" + /* * SubscriptingRefState.workspace for generic jsonb subscripting execution. * @@ -41,19 +43,10 @@ typedef struct JsonbSubWorkspace * JsonPathQuery() */ } JsonbSubWorkspace; -static Oid -jsonb_subscript_type(Node *expr) -{ - if (expr && IsA(expr, String)) - return TEXTOID; - - return exprType(expr); -} - static Node * coerce_jsonpath_subscript(ParseState *pstate, Node *subExpr, Oid numtype) { - Oid subExprType = jsonb_subscript_type(subExpr); + Oid subExprType = exprType(subExpr); Oid targetType = UNKNOWNOID; if (subExprType != UNKNOWNOID) @@ -110,12 +103,6 @@ coerce_jsonpath_subscript(ParseState *pstate, Node *subExpr, Oid numtype) COERCION_IMPLICIT, COERCE_IMPLICIT_CAST, -1); - if (subExpr == NULL) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("jsonb subscript must have text type"), - parser_errposition(pstate, exprLocation(subExpr)))); - return subExpr; } @@ -196,30 +183,22 @@ static JsonPathParseItem * make_jsonpath_item_expr(ParseState *pstate, Node *expr, List **exprs) { Const *cnst; + int32 val; expr = transformExpr(pstate, expr, pstate->p_expr_kind); - if (!IsA(expr, Const)) + if ((!IsA(expr, Const)) || (((Const *) expr)->consttype != INT4OID)) ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("jsonb simplified accessor supports subscripting in const int4, got type: %s", - format_type_be(exprType(expr))), - parser_errposition(pstate, exprLocation(expr)))); + errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("only integer constants are supported in jsonb simplified accessor subscripting"), + parser_errposition(pstate, exprLocation(expr))); cnst = (Const *) expr; + Assert(!cnst->constisnull); - if (cnst->consttype == INT4OID && !cnst->constisnull) - { - int32 val = DatumGetInt32(cnst->constvalue); + val = DatumGetInt32(cnst->constvalue); - return make_jsonpath_item_int(val, exprs); - } - - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("jsonb simplified accessor supports subscripting in type: INT4, got type: %s", - format_type_be(cnst->consttype)), - parser_errposition(pstate, exprLocation(expr)))); + return make_jsonpath_item_int(val, exprs); } /* @@ -239,14 +218,12 @@ make_jsonpath_item_expr(ParseState *pstate, Node *expr, List **exprs) * Parameters: * - pstate: Parse state context. * - indirection: List of subscripting expressions (modified in-place). - * - uexprs: Upper-bound expressions extracted from subscripts. - * - lexprs: Lower-bound expressions extracted from subscripts. - * Returns: - * - a Const node containing the transformed JsonPath expression. + * - sbsref: The SubscriptingRef node that some of its struct fields + * (refupperindexpr, reflowerindexpr, refjsonbpath) being populated. */ -static Node * +static void jsonb_subscript_make_jsonpath(ParseState *pstate, List **indirection, - List **uexprs, List **lexprs) + SubscriptingRef *sbsref) { JsonPathParseResult jpres; JsonPathParseItem *path = make_jsonpath_item(jpiRoot); @@ -254,9 +231,7 @@ jsonb_subscript_make_jsonpath(ParseState *pstate, List **indirection, Datum jsp; int pathlen = 0; - *uexprs = NIL; - *lexprs = NIL; - + Assert(sbsref->refjsonbpath == NULL); jpres.expr = path; jpres.lax = true; @@ -268,18 +243,30 @@ jsonb_subscript_make_jsonpath(ParseState *pstate, List **indirection, if (IsA(accessor, String)) { char *field = strVal(accessor); + Node *accessor_expr; jpi = make_jsonpath_item(jpiKey); jpi->value.string.val = field; jpi->value.string.len = strlen(field); - *uexprs = lappend(*uexprs, accessor); + /* + * jsonpath don't have collation, so here DEFAULT_COLLATION_OID + * should be fine. + * transformExpr can not cope with String node, so here we just + * simply make a text type Const. + */ + accessor_expr = (Node *) makeConst(TEXTOID, -1, DEFAULT_COLLATION_OID, -1, + CStringGetTextDatum(field), + false, + false); + + sbsref->refupperindexpr = lappend(sbsref->refupperindexpr, accessor_expr); } else if (IsA(accessor, A_Star)) { jpi = make_jsonpath_item(jpiAnyKey); - *uexprs = lappend(*uexprs, NULL); + sbsref->refupperindexpr = lappend(sbsref->refupperindexpr, NULL); } else if (IsA(accessor, A_Indices)) { @@ -291,26 +278,26 @@ jsonb_subscript_make_jsonpath(ParseState *pstate, List **indirection, if (ai->is_slice) { - while (list_length(*lexprs) < list_length(*uexprs)) - *lexprs = lappend(*lexprs, NULL); + while (list_length(sbsref->reflowerindexpr) < list_length(sbsref->refupperindexpr)) + sbsref->reflowerindexpr = lappend(sbsref->reflowerindexpr, NULL); - if (ai->lidx) - jpi->value.array.elems[0].from = make_jsonpath_item_expr(pstate, ai->lidx, lexprs); + if (ai->lidx != NULL) + jpi->value.array.elems[0].from = make_jsonpath_item_expr(pstate, ai->lidx, &sbsref->reflowerindexpr); else - jpi->value.array.elems[0].from = make_jsonpath_item_int(0, lexprs); + jpi->value.array.elems[0].from = make_jsonpath_item_int(0, &sbsref->reflowerindexpr); - if (ai->uidx) - jpi->value.array.elems[0].to = make_jsonpath_item_expr(pstate, ai->uidx, uexprs); + if (ai->uidx != NULL) + jpi->value.array.elems[0].to = make_jsonpath_item_expr(pstate, ai->uidx, &sbsref->refupperindexpr); else { jpi->value.array.elems[0].to = make_jsonpath_item(jpiLast); - *uexprs = lappend(*uexprs, NULL); + sbsref->refupperindexpr = lappend(sbsref->refupperindexpr, NULL); } } else { Assert(ai->uidx && !ai->lidx); - jpi->value.array.elems[0].from = make_jsonpath_item_expr(pstate, ai->uidx, uexprs); + jpi->value.array.elems[0].from = make_jsonpath_item_expr(pstate, ai->uidx, &sbsref->refupperindexpr); jpi->value.array.elems[0].to = NULL; } } @@ -323,17 +310,20 @@ jsonb_subscript_make_jsonpath(ParseState *pstate, List **indirection, pathlen++; } - if (*lexprs) + if (sbsref->reflowerindexpr != NIL) { - while (list_length(*lexprs) < list_length(*uexprs)) - *lexprs = lappend(*lexprs, NULL); + while (list_length(sbsref->reflowerindexpr) < list_length(sbsref->refupperindexpr)) + sbsref->reflowerindexpr = lappend(sbsref->reflowerindexpr, NULL); } + list_free(*indirection); + *indirection = NIL; + *indirection = list_delete_first_n(*indirection, pathlen); jsp = jsonPathFromParseResult(&jpres, 0, NULL); - return (Node *) makeConst(JSONPATHOID, -1, InvalidOid, -1, jsp, false, false); + sbsref->refjsonbpath = (Node *) makeConst(JSONPATHOID, -1, InvalidOid, -1, jsp, false, false); } /* @@ -358,10 +348,7 @@ jsonb_subscript_transform(SubscriptingRef *sbsref, if (jsonb_check_jsonpath_needed(*indirection)) { - sbsref->refjsonbpath = - jsonb_subscript_make_jsonpath(pstate, indirection, - &sbsref->refupperindexpr, - &sbsref->reflowerindexpr); + jsonb_subscript_make_jsonpath(pstate, indirection, sbsref); return; } @@ -629,7 +616,7 @@ jsonb_exec_setup(const SubscriptingRef *sbsref, { JsonbSubWorkspace *workspace; ListCell *lc; - int nupper = sbsref->refupperindexpr->length; + int nupper = list_length(sbsref->refupperindexpr); char *ptr; bool useJsonpath = sbsref->refjsonbpath != NULL; @@ -658,7 +645,7 @@ jsonb_exec_setup(const SubscriptingRef *sbsref, Node *expr = lfirst(lc); int i = foreach_current_index(lc); - workspace->indexOid[i] = jsonb_subscript_type(expr); + workspace->indexOid[i] = exprType(expr); } /* diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index c18e08dee29..97b79672641 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9334,6 +9334,10 @@ get_rule_expr(Node *node, deparse_context *context, */ need_parens = !IsA(sbsref->refexpr, Var) && !IsA(sbsref->refexpr, FieldSelect); + + if (sbsref->refjsonbpath != NULL) + need_parens = true; + if (need_parens) appendStringInfoChar(buf, '('); get_rule_expr((Node *) sbsref->refexpr, context, showimplicit); @@ -13008,10 +13012,15 @@ printSubscripts(SubscriptingRef *sbsref, deparse_context *context) { Node *up = (Node *) lfirst(uplist_item); - if (IsA(up, String)) + /* Dot Notatation can only be not-null text const */ + if ((up != NULL && IsA(up, Const) && + ((Const *) up)->consttype == TEXTOID)) { + Const *con = (Const *) up; + + Assert (!con->constisnull); appendStringInfoChar(buf, '.'); - appendStringInfoString(buf, quote_identifier(strVal(up))); + appendStringInfoString(buf, TextDatumGetCString(con->constvalue)); } else { diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out index 91a7b825764..5d536ea5c95 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -5163,7 +5163,7 @@ select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d']['a']; (1 row) select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb).d['a']; -ERROR: jsonb simplified accessor supports subscripting in type: INT4, got type: unknown +ERROR: only integer constants are supported in jsonb simplified accessor subscripting LINE 1: select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb).d['a']; ^ select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb).d.a; @@ -5233,7 +5233,7 @@ select ('{"a": ["a1", {"b1": ["aaa", "bbb", "ccc"]}], "b": "bb"}'::jsonb).a[1].b (1 row) select ('{"a": 1}'::jsonb)['a':'b']; -- fails -ERROR: jsonb simplified accessor supports subscripting in type: INT4, got type: unknown +ERROR: only integer constants are supported in jsonb simplified accessor subscripting LINE 1: select ('{"a": 1}'::jsonb)['a':'b']; ^ select ('[1, "2", null]'::jsonb)[1:2]; @@ -6038,7 +6038,7 @@ EXPLAIN (VERBOSE, COSTS OFF) SELECT (t.jb).a FROM test_jsonb_dot_notation t; QUERY PLAN ---------------------------------------------- Seq Scan on public.test_jsonb_dot_notation t - Output: jb.a + Output: (jb).a (2 rows) SELECT (t.jb).a FROM test_jsonb_dot_notation t; @@ -6051,7 +6051,7 @@ EXPLAIN (VERBOSE, COSTS OFF) SELECT (jb).a[1] FROM test_jsonb_dot_notation; QUERY PLAN -------------------------------------------- Seq Scan on public.test_jsonb_dot_notation - Output: jb.a[1] + Output: (jb).a[1] (2 rows) SELECT (jb).a[1] FROM test_jsonb_dot_notation; @@ -6060,6 +6060,22 @@ SELECT (jb).a[1] FROM test_jsonb_dot_notation; 2 (1 row) +CREATE VIEW v1 AS SELECT (jb).a[3].x.y FROM test_jsonb_dot_notation; +CREATE VIEW v2 AS SELECT (jb).a[3:].x.y[:-1] FROM test_jsonb_dot_notation; +CREATE VIEW v3 AS SELECT (jb).a[:3].x.y[-1:] FROM test_jsonb_dot_notation; +\sv v1 +CREATE OR REPLACE VIEW public.v1 AS + SELECT (jb).a[3].x.y AS y + FROM test_jsonb_dot_notation +\sv v2 +CREATE OR REPLACE VIEW public.v2 AS + SELECT (jb).a[3:].x.y[0:'-1'::integer] AS y + FROM test_jsonb_dot_notation +\sv v3 +CREATE OR REPLACE VIEW public.v3 AS + SELECT (jb).a[0:3].x.y['-1'::integer:] AS y + FROM test_jsonb_dot_notation +DROP VIEW v1, v2, v3; -- jsonb array access in plpgsql DO $$ DECLARE diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql index 4bd3990fb55..6aa71c7c3a4 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -1634,6 +1634,14 @@ SELECT (t.jb).a FROM test_jsonb_dot_notation t; EXPLAIN (VERBOSE, COSTS OFF) SELECT (jb).a[1] FROM test_jsonb_dot_notation; SELECT (jb).a[1] FROM test_jsonb_dot_notation; +CREATE VIEW v1 AS SELECT (jb).a[3].x.y FROM test_jsonb_dot_notation; +CREATE VIEW v2 AS SELECT (jb).a[3:].x.y[:-1] FROM test_jsonb_dot_notation; +CREATE VIEW v3 AS SELECT (jb).a[:3].x.y[-1:] FROM test_jsonb_dot_notation; +\sv v1 +\sv v2 +\sv v3 +DROP VIEW v1, v2, v3; + -- jsonb array access in plpgsql DO $$ DECLARE -- 2.34.1