Re: remaining sql/json patches
| От | Andres Freund |
|---|---|
| Тема | Re: remaining sql/json patches |
| Дата | |
| Msg-id | 20231208173014.2qycwc6kjqf6zlck@awork3.anarazel.de обсуждение исходный текст |
| Ответ на | Re: remaining sql/json patches (Amit Langote <amitlangote09@gmail.com>) |
| Ответы |
Re: remaining sql/json patches
(Amit Langote <amitlangote09@gmail.com>)
|
| Список | pgsql-hackers |
Hi, On 2023-12-07 21:07:59 +0900, Amit Langote wrote: > --- a/src/include/executor/execExpr.h > +++ b/src/include/executor/execExpr.h > @@ -16,6 +16,7 @@ > > #include "executor/nodeAgg.h" > #include "nodes/execnodes.h" > +#include "nodes/miscnodes.h" > > /* forward references to avoid circularity */ > struct ExprEvalStep; > @@ -168,6 +169,7 @@ typedef enum ExprEvalOp > > /* evaluate assorted special-purpose expression types */ > EEOP_IOCOERCE, > + EEOP_IOCOERCE_SAFE, > EEOP_DISTINCT, > EEOP_NOT_DISTINCT, > EEOP_NULLIF, > @@ -547,6 +549,7 @@ typedef struct ExprEvalStep > bool *checknull; > /* OID of domain type */ > Oid resulttype; > + ErrorSaveContext *escontext; > } domaincheck; > > /* for EEOP_CONVERT_ROWTYPE */ > @@ -776,6 +779,7 @@ extern void ExecEvalParamExec(ExprState *state, ExprEvalStep *op, > ExprContext *econtext); > extern void ExecEvalParamExtern(ExprState *state, ExprEvalStep *op, > ExprContext *econtext); > +extern void ExecEvalCoerceViaIOSafe(ExprState *state, ExprEvalStep *op); > extern void ExecEvalSQLValueFunction(ExprState *state, ExprEvalStep *op); > extern void ExecEvalCurrentOfExpr(ExprState *state, ExprEvalStep *op); > extern void ExecEvalNextValueExpr(ExprState *state, ExprEvalStep *op); > diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h > index 5d7f17dee0..6a7118d300 100644 > --- a/src/include/nodes/execnodes.h > +++ b/src/include/nodes/execnodes.h > @@ -34,6 +34,7 @@ > #include "fmgr.h" > #include "lib/ilist.h" > #include "lib/pairingheap.h" > +#include "nodes/miscnodes.h" > #include "nodes/params.h" > #include "nodes/plannodes.h" > #include "nodes/tidbitmap.h" > @@ -129,6 +130,12 @@ typedef struct ExprState > > Datum *innermost_domainval; > bool *innermost_domainnull; > + > + /* > + * For expression nodes that support soft errors. Should be set to NULL > + * before calling ExecInitExprRec() if the caller wants errors thrown. > + */ > + ErrorSaveContext *escontext; > } ExprState; Why do we need this both in ExprState *and* in ExprEvalStep? > From 38b53297b2d435d5cebf78c1f81e4748fed6c8b6 Mon Sep 17 00:00:00 2001 > From: Amit Langote <amitlan@postgresql.org> > Date: Wed, 22 Nov 2023 13:18:49 +0900 > Subject: [PATCH v30 2/5] Add soft error handling to populate_record_field() > > An uncoming patch would like the ability to call it from the > executor for some SQL/JSON expression nodes and ask to suppress any > errors that may occur. > > This commit does two things mainly: > > * It modifies the various interfaces internal to jsonfuncs.c to pass > the ErrorSaveContext around. > > * Make necessary modifications to handle the cases where the > processing is aborted partway through various functions that take > an ErrorSaveContext when a soft error occurs. > > Note that the above changes are only intended to suppress errors in > the functions in jsonfuncs.c, but not those in any external functions > that the functions in jsonfuncs.c in turn call, such as those from > arrayfuncs.c. It is assumed that the various populate_* functions > validate the data before passing those to external functions. > > Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com The code here is getting substantially more verbose / less readable. I wonder if there's something more general that could be improved to make this less painful? I'd not at all be surprised if this caused a measurable slowdown. > --- > src/backend/utils/adt/jsonfuncs.c | 310 +++++++++++++++++++++++------- > 1 file changed, 236 insertions(+), 74 deletions(-) > /* functions supporting jsonb_delete, jsonb_set and jsonb_concat */ > static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2, > @@ -2484,12 +2491,12 @@ populate_array_report_expected_array(PopulateArrayContext *ctx, int ndim) > if (ndim <= 0) > { > if (ctx->colname) > - ereport(ERROR, > + errsave(ctx->escontext, > (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), > errmsg("expected JSON array"), > errhint("See the value of key \"%s\".", ctx->colname))); > else > - ereport(ERROR, > + errsave(ctx->escontext, > (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), > errmsg("expected JSON array"))); > } > @@ -2506,13 +2513,13 @@ populate_array_report_expected_array(PopulateArrayContext *ctx, int ndim) > appendStringInfo(&indices, "[%d]", ctx->sizes[i]); > > if (ctx->colname) > - ereport(ERROR, > + errsave(ctx->escontext, > (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), > errmsg("expected JSON array"), > errhint("See the array element %s of key \"%s\".", > indices.data, ctx->colname))); > else > - ereport(ERROR, > + errsave(ctx->escontext, > (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), > errmsg("expected JSON array"), > errhint("See the array element %s.", > @@ -2520,8 +2527,13 @@ populate_array_report_expected_array(PopulateArrayContext *ctx, int ndim) > } > } It seems mildly errorprone to use errsave() but not have any returns in the code after the errsave()s - it seems plausible that somebody later would come and add more code expecting to not reach the later code. > +/* > + * Validate and set ndims for populating an array with some > + * populate_array_*() function. > + * > + * Returns false if the input (ndims) is erratic. I don't think "erratic" is the right word, "erroneous" maybe? > From 35cf1759f67a1c8ca7691aa87727a9f2c404b7c2 Mon Sep 17 00:00:00 2001 > From: Amit Langote <amitlan@postgresql.org> > Date: Tue, 5 Dec 2023 14:33:25 +0900 > Subject: [PATCH v30 3/5] SQL/JSON query functions > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > This introduces the SQL/JSON functions for querying JSON data using > jsonpath expressions. The functions are: > > JSON_EXISTS() > JSON_QUERY() > JSON_VALUE() > > JSON_EXISTS() tests if the jsonpath expression applied to the jsonb > value yields any values. > > JSON_VALUE() must return a single value, and an error occurs if it > tries to return multiple values. > > JSON_QUERY() must return a json object or array, and there are > various WRAPPER options for handling scalar or multi-value results. > Both these functions have options for handling EMPTY and ERROR > conditions. > > All of these functions only operate on jsonb. The workaround for now > is to cast the argument to jsonb. > > Author: Nikita Glukhov <n.gluhov@postgrespro.ru> > Author: Teodor Sigaev <teodor@sigaev.ru> > Author: Oleg Bartunov <obartunov@gmail.com> > Author: Alexander Korotkov <aekorotkov@gmail.com> > Author: Andrew Dunstan <andrew@dunslane.net> > Author: Amit Langote <amitlangote09@gmail.com> > Author: Peter Eisentraut <peter@eisentraut.org> > Author: jian he <jian.universality@gmail.com> > > Reviewers have included (in no particular order) Andres Freund, Alexander > Korotkov, Pavel Stehule, Andrew Alsup, Erik Rijkers, Zihong Yu, > Himanshu Upadhyaya, Daniel Gustafsson, Justin Pryzby, Álvaro Herrera, > jian he, Anton A. Melnikov, Nikita Malakhov, Peter Eisentraut > > Discussion: https://postgr.es/m/cd0bb935-0158-78a7-08b5-904886deac4b@postgrespro.ru > Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de > Discussion: https://postgr.es/m/abd9b83b-aa66-f230-3d6d-734817f0995d%40postgresql.org > Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com > --- > doc/src/sgml/func.sgml | 151 +++ > src/backend/catalog/sql_features.txt | 12 +- > src/backend/executor/execExpr.c | 363 +++++++ > src/backend/executor/execExprInterp.c | 365 ++++++- > src/backend/jit/llvm/llvmjit.c | 2 + > src/backend/jit/llvm/llvmjit_expr.c | 140 +++ > src/backend/jit/llvm/llvmjit_types.c | 4 + > src/backend/nodes/makefuncs.c | 18 + > src/backend/nodes/nodeFuncs.c | 238 ++++- > src/backend/optimizer/path/costsize.c | 3 +- > src/backend/optimizer/util/clauses.c | 19 + > src/backend/parser/gram.y | 178 +++- > src/backend/parser/parse_expr.c | 621 ++++++++++- > src/backend/parser/parse_target.c | 15 + > src/backend/utils/adt/formatting.c | 44 + > src/backend/utils/adt/jsonb.c | 31 + > src/backend/utils/adt/jsonfuncs.c | 52 +- > src/backend/utils/adt/jsonpath.c | 255 +++++ > src/backend/utils/adt/jsonpath_exec.c | 391 ++++++- > src/backend/utils/adt/ruleutils.c | 136 +++ > src/include/executor/execExpr.h | 133 +++ > src/include/fmgr.h | 1 + > src/include/jit/llvmjit.h | 1 + > src/include/nodes/makefuncs.h | 2 + > src/include/nodes/parsenodes.h | 47 + > src/include/nodes/primnodes.h | 130 +++ > src/include/parser/kwlist.h | 11 + > src/include/utils/formatting.h | 1 + > src/include/utils/jsonb.h | 1 + > src/include/utils/jsonfuncs.h | 5 + > src/include/utils/jsonpath.h | 27 + > src/interfaces/ecpg/preproc/ecpg.trailer | 28 + > src/test/regress/expected/json_sqljson.out | 18 + > src/test/regress/expected/jsonb_sqljson.out | 1032 +++++++++++++++++++ > src/test/regress/parallel_schedule | 2 +- > src/test/regress/sql/json_sqljson.sql | 11 + > src/test/regress/sql/jsonb_sqljson.sql | 337 ++++++ > src/tools/pgindent/typedefs.list | 18 + > 38 files changed, 4767 insertions(+), 76 deletions(-) I think it'd be worth trying to break this into smaller bits - it's not easy to review this at once. > +/* > + * Information about the state of JsonPath* evaluation. > + */ > +typedef struct JsonExprPostEvalState > +{ > + /* Did JsonPath* evaluation cause an error? */ > + NullableDatum error; > + > + /* Is the result of JsonPath* evaluation empty? */ > + NullableDatum empty; > + > + /* > + * ExecEvalJsonExprPath() will set this to the address of the step to > + * use to coerce the result of JsonPath* evaluation to the RETURNING type. > + * Also see the description of possible step addresses that this could be > + * set to in the definition of JsonExprState. > + */ > +#define FIELDNO_JSONEXPRPOSTEVALSTATE_JUMP_EVAL_COERCION 2 > + int jump_eval_coercion; > +} JsonExprPostEvalState; > + > +/* State for evaluating a JsonExpr, too big to inline */ > +typedef struct JsonExprState > +{ > + /* original expression node */ > + JsonExpr *jsexpr; > + > + /* value/isnull for formatted_expr */ > + NullableDatum formatted_expr; > + > + /* value/isnull for pathspec */ > + NullableDatum pathspec; > + > + /* JsonPathVariable entries for passing_values */ > + List *args; > + > + /* > + * Per-row result status info populated by ExecEvalJsonExprPath() > + * and ExecEvalJsonCoercionFinish(). > + */ > + JsonExprPostEvalState post_eval; > + > + /* > + * Address of the step that implements the non-ERROR variant of ON ERROR > + * and ON EMPTY behaviors, to be jumped to when ExecEvalJsonExprPath() > + * returns false on encountering an error during JsonPath* evaluation > + * (ON ERROR) or on finding that no matching JSON item was returned (ON > + * EMPTY). The same steps are also performed on encountering an error > + * when coercing JsonPath* result to the RETURNING type. > + */ > + int jump_error; > + > + /* > + * Addresses of steps to perform the coercion of the JsonPath* result value > + * to the RETURNING type. Each address points to either 1) a special > + * EEOP_JSONEXPR_COERCION step that handles coercion using the RETURNING > + * type's input function or by using json_via_populate(), or 2) an > + * expression such as CoerceViaIO. It may be -1 if no coercion is > + * necessary. > + * > + * jump_eval_result_coercion points to the step to evaluate the coercion > + * given in JsonExpr.result_coercion. > + */ > + int jump_eval_result_coercion; > + > + /* eval_item_coercion_jumps is an array of num_item_coercions elements > + * each containing a step address to evaluate the coercion from a value of > + * the given JsonItemType to the RETURNING type, or -1 if no coercion is > + * necessary. item_coercion_via_expr is an array of boolean flags of the > + * same length that indicates whether each valid step address in the > + * eval_item_coercion_jumps array points to an expression or a > + * EEOP_JSONEXPR_COERCION step. ExecEvalJsonExprPath() will cause an > + * error if it's the latter, because that mode of coercion is not > + * supported for all JsonItemTypes. > + */ > + int num_item_coercions; > + int *eval_item_coercion_jumps; > + bool *item_coercion_via_expr; > + > + /* > + * For passing when initializing a EEOP_IOCOERCE_SAFE step for any > + * CoerceViaIO nodes in the expression that must be evaluated in an > + * error-safe manner. > + */ > + ErrorSaveContext escontext; > +} JsonExprState; > + > +/* > + * State for coercing a value to the target type specified in 'coercion' using > + * either json_populate_type() or by calling the type's input function. > + */ > +typedef struct JsonCoercionState > +{ > + /* original expression node */ > + JsonCoercion *coercion; > + > + /* Input function info for the target type. */ > + struct > + { > + FmgrInfo *finfo; > + Oid typioparam; > + } input; > + > + /* Cache for json_populate_type() */ > + void *cache; > + > + /* > + * For soft-error handling in json_populate_type() or > + * in InputFunctionCallSafe(). > + */ > + ErrorSaveContext *escontext; > +} JsonCoercionState; Does all of this stuff need to live in this header? Some of it seems like it doesn't need to be in a header at all, and other bits seem like they belong somewhere more json specific? > +/* > + * JsonItemType > + * Represents type codes to identify a JsonCoercion node to use when > + * coercing a given SQL/JSON items to the output SQL type > + * > + * The comment next to each item type mentions the JsonbValue.jbvType of the > + * source JsonbValue value to be coerced using the expression in the > + * JsonCoercion node. > + * > + * Also, see InitJsonItemCoercions() and ExecPrepareJsonItemCoercion(). > + */ > +typedef enum JsonItemType > +{ > + JsonItemTypeNull = 0, /* jbvNull */ > + JsonItemTypeString = 1, /* jbvString */ > + JsonItemTypeNumeric = 2, /* jbvNumeric */ > + JsonItemTypeBoolean = 3, /* jbvBool */ > + JsonItemTypeDate = 4, /* jbvDatetime: DATEOID */ > + JsonItemTypeTime = 5, /* jbvDatetime: TIMEOID */ > + JsonItemTypeTimetz = 6, /* jbvDatetime: TIMETZOID */ > + JsonItemTypeTimestamp = 7, /* jbvDatetime: TIMESTAMPOID */ > + JsonItemTypeTimestamptz = 8, /* jbvDatetime: TIMESTAMPTZOID */ > + JsonItemTypeComposite = 9, /* jbvArray, jbvObject, jbvBinary */ > + JsonItemTypeInvalid = 10, > +} JsonItemType; Why do we need manually assigned values here? > +/* > + * JsonCoercion - > + * coercion from SQL/JSON item types to SQL types > + */ > +typedef struct JsonCoercion > +{ > + NodeTag type; > + > + Oid targettype; > + int32 targettypmod; > + bool via_populate; /* coerce result using json_populate_type()? */ > + bool via_io; /* coerce result using type input function? */ > + Oid collation; /* collation for coercion via I/O or populate */ > +} JsonCoercion; > + > +typedef struct JsonItemCoercion > +{ > + NodeTag type; > + > + JsonItemType item_type; > + Node *coercion; > +} JsonItemCoercion; What's the difference between an "ItemCoercion" and a "Coercion"? > +/* > + * JsonBehavior - > + * representation of a given JSON behavior My editor warns about space-before-tab here. > + */ > +typedef struct JsonBehavior > +{ > + NodeTag type; > + JsonBehaviorType btype; /* behavior type */ > + Node *expr; /* behavior expression */ These comment don't seem helpful. I think there's need for comments here, but restating the field name in different words isn't helpful. What's needed is an explanation of how things interact, perhaps also why that's the appropriate representation. > + JsonCoercion *coercion; /* to coerce behavior expression when there is > + * no cast to the target type */ > + int location; /* token location, or -1 if unknown */ > +} JsonBehavior; > + > +/* > + * JsonExpr - > + * transformed representation of JSON_VALUE(), JSON_QUERY(), JSON_EXISTS() > + */ > +typedef struct JsonExpr > +{ > + Expr xpr; > + > + JsonExprOp op; /* json function ID */ > + Node *formatted_expr; /* formatted context item expression */ > + Node *result_coercion; /* resulting coercion to RETURNING type */ > + JsonFormat *format; /* context item format (JSON/JSONB) */ > + Node *path_spec; /* JSON path specification expression */ > + List *passing_names; /* PASSING argument names */ > + List *passing_values; /* PASSING argument values */ > + JsonReturning *returning; /* RETURNING clause type/format info */ > + JsonBehavior *on_empty; /* ON EMPTY behavior */ > + JsonBehavior *on_error; /* ON ERROR behavior */ > + List *item_coercions; /* coercions for JSON_VALUE */ > + JsonWrapper wrapper; /* WRAPPER for JSON_QUERY */ > + bool omit_quotes; /* KEEP/OMIT QUOTES for JSON_QUERY */ > + int location; /* token location, or -1 if unknown */ > +} JsonExpr; These comments seem even worse. > +static void ExecInitJsonExpr(JsonExpr *jexpr, ExprState *state, > + Datum *resv, bool *resnull, > + ExprEvalStep *scratch); > +static int ExecInitJsonExprCoercion(ExprState *state, Node *coercion, > + ErrorSaveContext *escontext, > + Datum *resv, bool *resnull); > > > /* > @@ -2416,6 +2423,36 @@ ExecInitExprRec(Expr *node, ExprState *state, > break; > } > > + case T_JsonExpr: > + { > + JsonExpr *jexpr = castNode(JsonExpr, node); > + > + ExecInitJsonExpr(jexpr, state, resv, resnull, &scratch); > + break; > + } > + > + case T_JsonCoercion: > + { > + JsonCoercion *coercion = castNode(JsonCoercion, node); > + JsonCoercionState *jcstate = palloc0(sizeof(JsonCoercionState)); > + Oid typinput; > + FmgrInfo *finfo; > + > + getTypeInputInfo(coercion->targettype, &typinput, > + &jcstate->input.typioparam); > + finfo = palloc0(sizeof(FmgrInfo)); > + fmgr_info(typinput, finfo); > + jcstate->input.finfo = finfo; > + > + jcstate->coercion = coercion; > + jcstate->escontext = state->escontext; > + > + scratch.opcode = EEOP_JSONEXPR_COERCION; > + scratch.d.jsonexpr_coercion.jcstate = jcstate; > + ExprEvalPushStep(state, &scratch); > + break; > + } It's confusing that we have ExecInitJsonExprCoercion, but aren't using that here, but then use it later, in ExecInitJsonExpr(). > case T_NullTest: > { > NullTest *ntest = (NullTest *) node; > @@ -4184,3 +4221,329 @@ ExecBuildParamSetEqual(TupleDesc desc, > > return state; > } > + > +/* > + * Push steps to evaluate a JsonExpr and its various subsidiary expressions. > + */ > +static void > +ExecInitJsonExpr(JsonExpr *jexpr, ExprState *state, > + Datum *resv, bool *resnull, > + ExprEvalStep *scratch) > +{ > + JsonExprState *jsestate = palloc0(sizeof(JsonExprState)); > + JsonExprPostEvalState *post_eval = &jsestate->post_eval; > + ListCell *argexprlc; > + ListCell *argnamelc; > + List *jumps_if_skip = NIL; > + List *jumps_to_coerce_finish = NIL; > + List *jumps_to_end = NIL; > + ListCell *lc; > + ExprEvalStep *as; > + > + jsestate->jsexpr = jexpr; > + > + /* > + * Evaluate formatted_expr storing the result into > + * jsestate->formatted_expr. > + */ > + ExecInitExprRec((Expr *) jexpr->formatted_expr, state, > + &jsestate->formatted_expr.value, > + &jsestate->formatted_expr.isnull); > + > + /* Steps to jump to end if formatted_expr evaluates to NULL */ > + scratch->opcode = EEOP_JUMP_IF_NULL; > + scratch->resnull = &jsestate->formatted_expr.isnull; > + scratch->d.jump.jumpdone = -1; /* set below */ > + jumps_if_skip = lappend_int(jumps_if_skip, state->steps_len); > + ExprEvalPushStep(state, scratch); > + > + /* > + * Evaluate pathspec expression storing the result into > + * jsestate->pathspec. > + */ > + ExecInitExprRec((Expr *) jexpr->path_spec, state, > + &jsestate->pathspec.value, > + &jsestate->pathspec.isnull); > + > + /* Steps to JUMP to end if pathspec evaluates to NULL */ > + scratch->opcode = EEOP_JUMP_IF_NULL; > + scratch->resnull = &jsestate->pathspec.isnull; > + scratch->d.jump.jumpdone = -1; /* set below */ > + jumps_if_skip = lappend_int(jumps_if_skip, state->steps_len); > + ExprEvalPushStep(state, scratch); > + > + /* Steps to compute PASSING args. */ > + jsestate->args = NIL; > + forboth(argexprlc, jexpr->passing_values, > + argnamelc, jexpr->passing_names) > + { > + Expr *argexpr = (Expr *) lfirst(argexprlc); > + String *argname = lfirst_node(String, argnamelc); > + JsonPathVariable *var = palloc(sizeof(*var)); > + > + var->name = argname->sval; > + var->typid = exprType((Node *) argexpr); > + var->typmod = exprTypmod((Node *) argexpr); > + > + ExecInitExprRec((Expr *) argexpr, state, &var->value, &var->isnull); > + > + jsestate->args = lappend(jsestate->args, var); > + } > + > + /* Step for JsonPath* evaluation; see ExecEvalJsonExprPath(). */ > + scratch->opcode = EEOP_JSONEXPR_PATH; > + scratch->resvalue = resv; > + scratch->resnull = resnull; > + scratch->d.jsonexpr.jsestate = jsestate; > + ExprEvalPushStep(state, scratch); > + > + /* > + * Step to jump to end when there's neither an error when evaluating > + * JsonPath* nor any need to coerce the result because it's already > + * of the specified type. > + */ > + scratch->opcode = EEOP_JUMP; > + scratch->d.jump.jumpdone = -1; /* set below */ > + jumps_to_end = lappend_int(jumps_to_end, state->steps_len); > + ExprEvalPushStep(state, scratch); > + > + /* > + * Steps to coerce the result value computed by EEOP_JSONEXPR_PATH. > + * To handle coercion errors softly, use the following ErrorSaveContext > + * when initializing the coercion expressions, including any JsonCoercion > + * nodes. > + */ > + jsestate->escontext.type = T_ErrorSaveContext; > + if (jexpr->result_coercion || jexpr->omit_quotes) > + { > + jsestate->jump_eval_result_coercion = > + ExecInitJsonExprCoercion(state, jexpr->result_coercion, > + jexpr->on_error->btype != JSON_BEHAVIOR_ERROR ? > + &jsestate->escontext : NULL, > + resv, resnull); > + } > + else > + jsestate->jump_eval_result_coercion = -1; > + > + /* Steps for coercing JsonItemType values returned by JsonPathValue(). */ > + if (jexpr->item_coercions) > + { > + /* > + * Jump to COERCION_FINISH to skip over the following steps if > + * result_coercion is present. > + */ > + if (jsestate->jump_eval_result_coercion >= 0) > + { > + scratch->opcode = EEOP_JUMP; > + scratch->d.jump.jumpdone = -1; /* set below */ > + jumps_to_coerce_finish = lappend_int(jumps_to_coerce_finish, > + state->steps_len); > + ExprEvalPushStep(state, scratch); > + } > + > + /* > + * Here we create the steps for each JsonItemType type's coercion > + * expression and also store a flag whether the expression is > + * a JsonCoercion node. ExecPrepareJsonItemCoercion() called by > + * ExecEvalJsonExprPath() will map a given JsonbValue returned by > + * JsonPathValue() to its JsonItemType's expression's step address > + * and the flag by indexing the following arrays with JsonItemType > + * enum value. > + */ > + jsestate->num_item_coercions = list_length(jexpr->item_coercions); > + jsestate->eval_item_coercion_jumps = (int *) > + palloc(jsestate->num_item_coercions * sizeof(int)); > + jsestate->item_coercion_via_expr = (bool *) > + palloc0(jsestate->num_item_coercions * sizeof(bool)); > + foreach(lc, jexpr->item_coercions) > + { > + JsonItemCoercion *item_coercion = lfirst(lc); > + Node *coercion = item_coercion->coercion; > + > + jsestate->item_coercion_via_expr[item_coercion->item_type] = > + (coercion != NULL && !IsA(coercion, JsonCoercion)); > + jsestate->eval_item_coercion_jumps[item_coercion->item_type] = > + ExecInitJsonExprCoercion(state, coercion, > + jexpr->on_error->btype != JSON_BEHAVIOR_ERROR ? > + &jsestate->escontext : NULL, > + resv, resnull); > + > + /* Emit JUMP step to skip past other coercions' steps. */ > + scratch->opcode = EEOP_JUMP; > + scratch->d.jump.jumpdone = -1; /* set below */ > + jumps_to_coerce_finish = lappend_int(jumps_to_coerce_finish, > + state->steps_len); > + ExprEvalPushStep(state, scratch); > + } > + } > + > + /* > + * Add step to reset the ErrorSaveContext and set error flag if the > + * coercion steps encountered an error but was not thrown because of the > + * ON ERROR behavior. > + */ > + if (jexpr->result_coercion || jexpr->item_coercions) > + { > + foreach(lc, jumps_to_coerce_finish) > + { > + as = &state->steps[lfirst_int(lc)]; > + as->d.jump.jumpdone = state->steps_len; > + } > + > + scratch->opcode = EEOP_JSONEXPR_COERCION_FINISH; > + scratch->d.jsonexpr.jsestate = jsestate; > + ExprEvalPushStep(state, scratch); > + } > + > + /* > + * Step to handle ON ERROR behaviors. This handles both the errors > + * that occur during EEOP_JSONEXPR_PATH evaluation and subsequent coercion > + * evaluation. > + */ > + jsestate->jump_error = -1; > + if (jexpr->on_error && > + jexpr->on_error->btype != JSON_BEHAVIOR_ERROR) > + { > + jsestate->jump_error = state->steps_len; > + scratch->opcode = EEOP_JUMP_IF_NOT_TRUE; > + > + /* > + * post_eval.error is set by ExecEvalJsonExprPath() and > + * ExecEvalJsonCoercionFinish(). > + */ > + scratch->resvalue = &post_eval->error.value; > + scratch->resnull = &post_eval->error.isnull; > + > + scratch->d.jump.jumpdone = -1; /* set below */ > + ExprEvalPushStep(state, scratch); > + > + /* Steps to evaluate the ON ERROR expression */ > + ExecInitExprRec((Expr *) jexpr->on_error->expr, > + state, resv, resnull); > + > + /* Steps to coerce the ON ERROR expression if needed */ > + if (jexpr->on_error->coercion) > + ExecInitExprRec((Expr *) jexpr->on_error->coercion, state, > + resv, resnull); > + > + jumps_to_end = lappend_int(jumps_to_end, state->steps_len); > + scratch->opcode = EEOP_JUMP; > + scratch->d.jump.jumpdone = -1; > + ExprEvalPushStep(state, scratch); > + } > + > + /* Step to handle ON EMPTY behaviors. */ > + if (jexpr->on_empty != NULL && > + jexpr->on_empty->btype != JSON_BEHAVIOR_ERROR) > + { > + /* > + * Make the ON ERROR behavior JUMP to here after checking the error > + * and if it's not present then make EEOP_JSONEXPR_PATH directly > + * jump here. > + */ > + if (jsestate->jump_error >= 0) > + { > + as = &state->steps[jsestate->jump_error]; > + as->d.jump.jumpdone = state->steps_len; > + } > + else > + jsestate->jump_error = state->steps_len; > + > + scratch->opcode = EEOP_JUMP_IF_NOT_TRUE; > + scratch->resvalue = &post_eval->empty.value; > + scratch->resnull = &post_eval->empty.isnull; > + scratch->d.jump.jumpdone = -1; /* set below */ > + jumps_to_end = lappend_int(jumps_to_end, state->steps_len); > + ExprEvalPushStep(state, scratch); > + > + /* Steps to evaluate the ON EMPTY expression */ > + ExecInitExprRec((Expr *) jexpr->on_empty->expr, > + state, resv, resnull); > + > + /* Steps to coerce the ON EMPTY expression if needed */ > + if (jexpr->on_empty->coercion) > + ExecInitExprRec((Expr *) jexpr->on_empty->coercion, state, > + resv, resnull); > + > + scratch->opcode = EEOP_JUMP; > + scratch->d.jump.jumpdone = -1; /* set below */ > + jumps_to_end = lappend_int(jumps_to_end, state->steps_len); > + ExprEvalPushStep(state, scratch); > + } > + /* Make EEOP_JSONEXPR_PATH jump to end if no ON EMPTY clause present. */ > + else if (jsestate->jump_error >= 0) > + jumps_to_end = lappend_int(jumps_to_end, jsestate->jump_error); > + > + /* > + * If neither ON ERROR nor ON EMPTY jumps present, then add one to go > + * to end. > + */ > + if (jsestate->jump_error < 0) > + { > + scratch->opcode = EEOP_JUMP; > + scratch->d.jump.jumpdone = -1; /* set below */ > + jumps_to_end = lappend_int(jumps_to_end, state->steps_len); > + ExprEvalPushStep(state, scratch); > + } > + > + /* Return NULL when either formatted_expr or pathspec is NULL. */ > + foreach(lc, jumps_if_skip) > + { > + as = &state->steps[lfirst_int(lc)]; > + as->d.jump.jumpdone = state->steps_len; > + } > + scratch->opcode = EEOP_CONST; > + scratch->resvalue = resv; > + scratch->resnull = resnull; > + scratch->d.constval.value = (Datum) 0; > + scratch->d.constval.isnull = true; > + ExprEvalPushStep(state, scratch); > + > + /* Jump to coerce the NULL using result_coercion is present. */ > + if (jsestate->jump_eval_result_coercion >= 0) > + { > + scratch->opcode = EEOP_JUMP; > + scratch->d.jump.jumpdone = jsestate->jump_eval_result_coercion; > + ExprEvalPushStep(state, scratch); > + } > + > + foreach(lc, jumps_to_end) > + { > + as = &state->steps[lfirst_int(lc)]; > + as->d.jump.jumpdone = state->steps_len; > + } > +} > + > +/* Initialize one JsonCoercion for execution. */ > +static int > +ExecInitJsonExprCoercion(ExprState *state, Node *coercion, > + ErrorSaveContext *escontext, > + Datum *resv, bool *resnull) > +{ > + int jump_eval_coercion; > + Datum *save_innermost_caseval; > + bool *save_innermost_casenull; > + ErrorSaveContext *save_escontext; > + > + if (coercion == NULL) > + return -1; > + > + jump_eval_coercion = state->steps_len; > + > + /* Push step(s) to compute cstate->coercion. */ > + save_innermost_caseval = state->innermost_caseval; > + save_innermost_casenull = state->innermost_casenull; > + save_escontext = state->escontext; > + > + state->innermost_caseval = resv; > + state->innermost_casenull = resnull; > + state->escontext = escontext; > + > + ExecInitExprRec((Expr *) coercion, state, resv, resnull); > + > + state->innermost_caseval = save_innermost_caseval; > + state->innermost_casenull = save_innermost_casenull; > + state->escontext = save_escontext; > + > + return jump_eval_coercion; > +} > diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c > index d5db96444c..a18662cbf9 100644 > --- a/src/backend/executor/execExprInterp.c > +++ b/src/backend/executor/execExprInterp.c > @@ -73,8 +73,8 @@ > #include "utils/datum.h" > #include "utils/expandedrecord.h" > #include "utils/json.h" > -#include "utils/jsonb.h" > #include "utils/jsonfuncs.h" > +#include "utils/jsonpath.h" > #include "utils/lsyscache.h" > #include "utils/memutils.h" > #include "utils/timestamp.h" > @@ -181,6 +181,10 @@ static pg_attribute_always_inline void ExecAggPlainTransByRef(AggState *aggstate > AggStatePerGroup pergroup, > ExprContext *aggcontext, > int setno); > +static void ExecPrepareJsonItemCoercion(JsonbValue *item, JsonExprState *jsestate, > + bool throw_error, > + int *jump_eval_item_coercion, > + Datum *resvalue, bool *resnull); > > /* > * ScalarArrayOpExprHashEntry > @@ -482,6 +486,9 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) > &&CASE_EEOP_XMLEXPR, > &&CASE_EEOP_JSON_CONSTRUCTOR, > &&CASE_EEOP_IS_JSON, > + &&CASE_EEOP_JSONEXPR_PATH, > + &&CASE_EEOP_JSONEXPR_COERCION, > + &&CASE_EEOP_JSONEXPR_COERCION_FINISH, > &&CASE_EEOP_AGGREF, > &&CASE_EEOP_GROUPING_FUNC, > &&CASE_EEOP_WINDOW_FUNC, > @@ -1551,6 +1558,35 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) > EEO_NEXT(); > } > > + EEO_CASE(EEOP_JSONEXPR_PATH) > + { > + JsonExprState *jsestate = op->d.jsonexpr.jsestate; > + > + /* too complex for an inline implementation */ > + if (!ExecEvalJsonExprPath(state, op, econtext)) > + EEO_JUMP(jsestate->jump_error); > + else if (jsestate->post_eval.jump_eval_coercion >= 0) > + EEO_JUMP(jsestate->post_eval.jump_eval_coercion); > + > + EEO_NEXT(); > + } Why do we need post_eval.jump_eval_coercion? Seems like that could more cleanly be implemented by just emitting a constant JUMP step? Oh, I see - you're changing post_eval.jump_eval_coercion at runtime. This seems like a BAD idea. I strongly suggest that instead of modifying the field, you instead return the target jump step as a return value from ExecEvalJsonExprPath or such. > +/* > + * Performs JsonPath{Exists|Query|Value}() for given context item and JSON > + * path. > + * > + * Result is set in *op->resvalue and *op->resnull. > + * > + * On return, JsonExprPostEvalState is populated with the following details: > + * - jump_eval_coercion: step address of coercion to apply to the result > + * - error.value: true if an error occurred during JsonPath evaluation > + * - empty.value: true if JsonPath{Query|Value}() found no matching item > + * > + * No return if the ON ERROR/EMPTY behavior is ERROR. > + */ > +bool > +ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op, > + ExprContext *econtext) > +{ > + JsonExprState *jsestate = op->d.jsonexpr.jsestate; > + JsonExprPostEvalState *post_eval = &jsestate->post_eval; > + JsonExpr *jexpr = jsestate->jsexpr; > + Datum item; > + JsonPath *path; > + bool throw_error = (jexpr->on_error->btype == JSON_BEHAVIOR_ERROR); What's the deal with the parentheses here and in similar places below? There's no danger of ambiguity without, no? > + case JSON_VALUE_OP: > + { > + JsonbValue *jbv = JsonPathValue(item, path, &empty, > + !throw_error ? &error : NULL, > + jsestate->args); > + > + /* Might get overridden by an item coercion below. */ > + post_eval->jump_eval_coercion = jsestate->jump_eval_result_coercion; > + if (jbv == NULL) > + { > + /* Will be coerced with result_coercion. */ > + *op->resvalue = (Datum) 0; > + *op->resnull = true; > + } > + else if (!error && !empty) > + { > + /* > + * If the requested output type is json(b), use > + * result_coercion to do the coercion. > + */ > + if (jexpr->returning->typid == JSONOID || > + jexpr->returning->typid == JSONBOID) > + { > + *op->resvalue = JsonbPGetDatum(JsonbValueToJsonb(jbv)); > + *op->resnull = false; > + } > + else > + { > + /* > + * Else, use one of the item_coercions. > + * > + * Error out if no cast expression exists. > + */ > + ExecPrepareJsonItemCoercion(jbv, jsestate, throw_error, > + &post_eval->jump_eval_coercion, > + op->resvalue, op->resnull); > + if (empty) > + { > + if (jexpr->on_empty) > + { > + if (jexpr->on_empty->btype == JSON_BEHAVIOR_ERROR) > + ereport(ERROR, > + (errcode(ERRCODE_NO_SQL_JSON_ITEM), > + errmsg("no SQL/JSON item"))); No need for the parens around ereport() arguments anymore. Same in a few other places. > diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y > index d631ac89a9..4f92d000ec 100644 > --- a/src/backend/parser/gram.y > +++ b/src/backend/parser/gram.y > @@ -650,11 +650,18 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); > json_returning_clause_opt > json_name_and_value > json_aggregate_func > + json_argument > + json_behavior > %type <list> json_name_and_value_list > json_value_expr_list > json_array_aggregate_order_by_clause_opt > + json_arguments > + json_behavior_clause_opt > + json_passing_clause_opt > %type <ival> json_encoding_clause_opt > json_predicate_type_constraint > + json_quotes_clause_opt > + json_wrapper_behavior > %type <boolean> json_key_uniqueness_constraint_opt > json_object_constructor_null_clause_opt > json_array_constructor_null_clause_opt > @@ -695,7 +702,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); > CACHE CALL CALLED CASCADE CASCADED CASE CAST CATALOG_P CHAIN CHAR_P > CHARACTER CHARACTERISTICS CHECK CHECKPOINT CLASS CLOSE > CLUSTER COALESCE COLLATE COLLATION COLUMN COLUMNS COMMENT COMMENTS COMMIT > - COMMITTED COMPRESSION CONCURRENTLY CONFIGURATION CONFLICT > + COMMITTED COMPRESSION CONCURRENTLY CONDITIONAL CONFIGURATION CONFLICT > CONNECTION CONSTRAINT CONSTRAINTS CONTENT_P CONTINUE_P CONVERSION_P COPY > COST CREATE CROSS CSV CUBE CURRENT_P > CURRENT_CATALOG CURRENT_DATE CURRENT_ROLE CURRENT_SCHEMA > @@ -706,8 +713,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); > DETACH DICTIONARY DISABLE_P DISCARD DISTINCT DO DOCUMENT_P DOMAIN_P > DOUBLE_P DROP > > - EACH ELSE ENABLE_P ENCODING ENCRYPTED END_P ENUM_P ESCAPE EVENT EXCEPT > - EXCLUDE EXCLUDING EXCLUSIVE EXECUTE EXISTS EXPLAIN EXPRESSION > + EACH ELSE EMPTY_P ENABLE_P ENCODING ENCRYPTED END_P ENUM_P ERROR_P ESCAPE > + EVENT EXCEPT EXCLUDE EXCLUDING EXCLUSIVE EXECUTE EXISTS EXPLAIN EXPRESSION > EXTENSION EXTERNAL EXTRACT > > FALSE_P FAMILY FETCH FILTER FINALIZE FIRST_P FLOAT_P FOLLOWING FOR > @@ -722,10 +729,10 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); > INNER_P INOUT INPUT_P INSENSITIVE INSERT INSTEAD INT_P INTEGER > INTERSECT INTERVAL INTO INVOKER IS ISNULL ISOLATION > > - JOIN JSON JSON_ARRAY JSON_ARRAYAGG JSON_OBJECT JSON_OBJECTAGG > - JSON_SCALAR JSON_SERIALIZE > + JOIN JSON JSON_ARRAY JSON_ARRAYAGG JSON_EXISTS JSON_OBJECT JSON_OBJECTAGG > + JSON_QUERY JSON_SCALAR JSON_SERIALIZE JSON_VALUE > > - KEY KEYS > + KEEP KEY KEYS > > LABEL LANGUAGE LARGE_P LAST_P LATERAL_P > LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL > @@ -739,7 +746,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); > NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF > NULLS_P NUMERIC > > - OBJECT_P OF OFF OFFSET OIDS OLD ON ONLY OPERATOR OPTION OPTIONS OR > + OBJECT_P OF OFF OFFSET OIDS OLD OMIT ON ONLY OPERATOR OPTION OPTIONS OR > ORDER ORDINALITY OTHERS OUT_P OUTER_P > OVER OVERLAPS OVERLAY OVERRIDING OWNED OWNER > > @@ -748,7 +755,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); > POSITION PRECEDING PRECISION PRESERVE PREPARE PREPARED PRIMARY > PRIOR PRIVILEGES PROCEDURAL PROCEDURE PROCEDURES PROGRAM PUBLICATION > > - QUOTE > + QUOTE QUOTES > > RANGE READ REAL REASSIGN RECHECK RECURSIVE REF_P REFERENCES REFERENCING > REFRESH REINDEX RELATIVE_P RELEASE RENAME REPEATABLE REPLACE REPLICA > @@ -759,7 +766,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); > SEQUENCE SEQUENCES > SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW > SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P > - START STATEMENT STATISTICS STDIN STDOUT STORAGE STORED STRICT_P STRIP_P > + START STATEMENT STATISTICS STDIN STDOUT STORAGE STORED STRICT_P STRING_P STRIP_P > SUBSCRIPTION SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P SYSTEM_USER > > TABLE TABLES TABLESAMPLE TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN > @@ -767,7 +774,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); > TREAT TRIGGER TRIM TRUE_P > TRUNCATE TRUSTED TYPE_P TYPES_P > > - UESCAPE UNBOUNDED UNCOMMITTED UNENCRYPTED UNION UNIQUE UNKNOWN > + UESCAPE UNBOUNDED UNCONDITIONAL UNCOMMITTED UNENCRYPTED UNION UNIQUE UNKNOWN > UNLISTEN UNLOGGED UNTIL UPDATE USER USING > > VACUUM VALID VALIDATE VALIDATOR VALUE_P VALUES VARCHAR VARIADIC VARYING > @@ -15768,6 +15775,60 @@ func_expr_common_subexpr: > n->location = @1; > $$ = (Node *) n; > } > + | JSON_QUERY '(' > + json_value_expr ',' a_expr json_passing_clause_opt > + json_returning_clause_opt > + json_wrapper_behavior > + json_quotes_clause_opt > + json_behavior_clause_opt > + ')' > + { > + JsonFuncExpr *n = makeNode(JsonFuncExpr); > + > + n->op = JSON_QUERY_OP; > + n->context_item = (JsonValueExpr *) $3; > + n->pathspec = $5; > + n->passing = $6; > + n->output = (JsonOutput *) $7; > + n->wrapper = $8; > + n->quotes = $9; > + n->behavior = $10; > + n->location = @1; > + $$ = (Node *) n; > + } > + | JSON_EXISTS '(' > + json_value_expr ',' a_expr json_passing_clause_opt > + json_behavior_clause_opt > + ')' > + { > + JsonFuncExpr *n = makeNode(JsonFuncExpr); > + > + n->op = JSON_EXISTS_OP; > + n->context_item = (JsonValueExpr *) $3; > + n->pathspec = $5; > + n->passing = $6; > + n->output = NULL; > + n->behavior = $7; > + n->location = @1; > + $$ = (Node *) n; > + } > + | JSON_VALUE '(' > + json_value_expr ',' a_expr json_passing_clause_opt > + json_returning_clause_opt > + json_behavior_clause_opt > + ')' > + { > + JsonFuncExpr *n = makeNode(JsonFuncExpr); > + > + n->op = JSON_VALUE_OP; > + n->context_item = (JsonValueExpr *) $3; > + n->pathspec = $5; > + n->passing = $6; > + n->output = (JsonOutput *) $7; > + n->behavior = $8; > + n->location = @1; > + $$ = (Node *) n; > + } > ; > > > @@ -16494,6 +16555,27 @@ opt_asymmetric: ASYMMETRIC > ; > > /* SQL/JSON support */ > +json_passing_clause_opt: > + PASSING json_arguments { $$ = $2; } > + | /*EMPTY*/ { $$ = NIL; } > + ; > + > +json_arguments: > + json_argument { $$ = list_make1($1); } > + | json_arguments ',' json_argument { $$ = lappend($1, $3); } > + ; > + > +json_argument: > + json_value_expr AS ColLabel > + { > + JsonArgument *n = makeNode(JsonArgument); > + > + n->val = (JsonValueExpr *) $1; > + n->name = $3; > + $$ = (Node *) n; > + } > + ; > + > json_value_expr: > a_expr json_format_clause_opt > { > @@ -16519,6 +16601,27 @@ json_encoding_clause_opt: > | /* EMPTY */ { $$ = JS_ENC_DEFAULT; } > ; > > +/* ARRAY is a noise word */ > +json_wrapper_behavior: > + WITHOUT WRAPPER { $$ = JSW_NONE; } > + | WITHOUT ARRAY WRAPPER { $$ = JSW_NONE; } > + | WITH WRAPPER { $$ = JSW_UNCONDITIONAL; } > + | WITH ARRAY WRAPPER { $$ = JSW_UNCONDITIONAL; } > + | WITH CONDITIONAL ARRAY WRAPPER { $$ = JSW_CONDITIONAL; } > + | WITH UNCONDITIONAL ARRAY WRAPPER { $$ = JSW_UNCONDITIONAL; } > + | WITH CONDITIONAL WRAPPER { $$ = JSW_CONDITIONAL; } > + | WITH UNCONDITIONAL WRAPPER { $$ = JSW_UNCONDITIONAL; } > + | /* empty */ { $$ = JSW_NONE; } > + ; > + > +json_quotes_clause_opt: > + KEEP QUOTES ON SCALAR STRING_P { $$ = JS_QUOTES_KEEP; } > + | KEEP QUOTES { $$ = JS_QUOTES_KEEP; } > + | OMIT QUOTES ON SCALAR STRING_P { $$ = JS_QUOTES_OMIT; } > + | OMIT QUOTES { $$ = JS_QUOTES_OMIT; } > + | /* EMPTY */ { $$ = JS_QUOTES_UNSPEC; } > + ; > + > json_returning_clause_opt: > RETURNING Typename json_format_clause_opt > { > @@ -16532,6 +16635,39 @@ json_returning_clause_opt: > | /* EMPTY */ { $$ = NULL; } > ; > > +json_behavior: > + DEFAULT a_expr > + { $$ = (Node *) makeJsonBehavior(JSON_BEHAVIOR_DEFAULT, $2, NULL, @1); } > + | ERROR_P > + { $$ = (Node *) makeJsonBehavior(JSON_BEHAVIOR_ERROR, NULL, NULL, @1); } > + | NULL_P > + { $$ = (Node *) makeJsonBehavior(JSON_BEHAVIOR_NULL, NULL, NULL, @1); } > + | TRUE_P > + { $$ = (Node *) makeJsonBehavior(JSON_BEHAVIOR_TRUE, NULL, NULL, @1); } > + | FALSE_P > + { $$ = (Node *) makeJsonBehavior(JSON_BEHAVIOR_FALSE, NULL, NULL, @1); } > + | UNKNOWN > + { $$ = (Node *) makeJsonBehavior(JSON_BEHAVIOR_UNKNOWN, NULL, NULL, @1); } > + | EMPTY_P ARRAY > + { $$ = (Node *) makeJsonBehavior(JSON_BEHAVIOR_EMPTY_ARRAY, NULL, NULL, @1); } > + | EMPTY_P OBJECT_P > + { $$ = (Node *) makeJsonBehavior(JSON_BEHAVIOR_EMPTY_OBJECT, NULL, NULL, @1); } > + /* non-standard, for Oracle compatibility only */ > + | EMPTY_P > + { $$ = (Node *) makeJsonBehavior(JSON_BEHAVIOR_EMPTY_ARRAY, NULL, NULL, @1); } > + ; Seems like this would look better if you made json_behavior return just the enum values and had one makeJsonBehavior() at the place referencing it? > +json_behavior_clause_opt: > + json_behavior ON EMPTY_P > + { $$ = list_make2($1, NULL); } > + | json_behavior ON ERROR_P > + { $$ = list_make2(NULL, $1); } > + | json_behavior ON EMPTY_P json_behavior ON ERROR_P > + { $$ = list_make2($1, $4); } > + | /* EMPTY */ > + { $$ = list_make2(NULL, NULL); } > + ; This seems like an odd representation - why represent the behavior as a two element list where one needs to know what is stored at which list offset? Greetings, Andres Freund
В списке pgsql-hackers по дате отправления:
Следующее
От: Alexander KorotkovДата:
Сообщение: Re: Assert failure on 'list_member_ptr(rel->joininfo, restrictinfo)'