public inbox for [email protected]  

help / color / mirror / Atom feed

From: jian he <[email protected]>
To: Amit Langote <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Himanshu Upadhyaya <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Erik Rijkers <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: remaining sql/json patches
Date: Wed, 20 Mar 2024 14:51:48 +0800
Message-ID: <CACJufxFkKV8-tMa88ByaabYDQan9+cFCcf1Z1jH7=bQuMAHN4A@mail.gmail.com> (raw)
In-Reply-To: <CACJufxE4NDVY_Xeg=ip96MBw2u-U=qe6ar2qo2D2PQm-vDeDaQ@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CA+HiwqGtdVCg-KboUUU=svwukwXGT4EOFOk3eCZLz6h-d6YLNw@mail.gmail.com>
	<CAPF61jAOJM6FqMsPGYHXbKxtX8DQ2a7kuOZ3AzR8oGXHy3fjFA@mail.gmail.com>
	<[email protected]>
	<CAPF61jBm+_QCwDg32_K+BgNKf09isU9g3o8CLW2kUDLZ5=28Hw@mail.gmail.com>
	<[email protected]>
	<CA+HiwqF5CHyRBO8WwOcQsKufFWZ1yjDaQQ7DJxsQNbm2nab3pg@mail.gmail.com>
	<CA+HiwqHjQcQaUx63Z65KnBdDg=xWiKKH84zNL=T+Gg4bKPS8dw@mail.gmail.com>
	<CACJufxE4NDVY_Xeg=ip96MBw2u-U=qe6ar2qo2D2PQm-vDeDaQ@mail.gmail.com>

minor issues I found while looking through it.
other than these issues, looks good!

/*
 * Convert the a given JsonbValue to its C string representation
 *
 * Returns the string as a Datum setting *resnull if the JsonbValue is a
 * a jbvNull.
 */
static char *
ExecGetJsonValueItemString(JsonbValue *item, bool *resnull)
{
}
I think the comments are not right?

/*
 * Checks if the coercion evaluation led to an error.  If an error did occur,
 * this sets post_eval->error to trigger the ON ERROR handling steps.
 */
void
ExecEvalJsonCoercionFinish(ExprState *state, ExprEvalStep *op)
{
}
these comments on ExecEvalJsonCoercionFinish also need to be updated?


+ /*
+ * Coerce the result value by calling the input function coercion.
+ * *op->resvalue must point to C string in this case.
+ */
+ if (!*op->resnull && jsexpr->use_io_coercion)
+ {
+ FunctionCallInfo fcinfo;
+
+ fcinfo = jsestate->input_fcinfo;
+ Assert(fcinfo != NULL);
+ Assert(val_string != NULL);
+ fcinfo->args[0].value = PointerGetDatum(val_string);
+ fcinfo->args[0].isnull = *op->resnull;
+ /* second and third arguments are already set up */
+
+ fcinfo->isnull = false;
+ *op->resvalue = FunctionCallInvoke(fcinfo);
+ if (SOFT_ERROR_OCCURRED(&jsestate->escontext))
+ error = true;
+
+ jump_eval_coercion = -1;
+ }

+ /* second and third arguments are already set up */
change to
/* second and third arguments are already set up in ExecInitJsonExpr */
would be great.


commit message
<<<<
All of these functions only operate on jsonb values. The workaround
for now is to cast the argument to jsonb.
<<<<
should be removed?


+ case T_JsonFuncExpr:
+ {
+ JsonFuncExpr *jfe = (JsonFuncExpr *) node;
+
+ if (WALK(jfe->context_item))
+ return true;
+ if (WALK(jfe->pathspec))
+ return true;
+ if (WALK(jfe->passing))
+ return true;
+ if (jfe->output && WALK(jfe->output))
+ return true;
+ if (jfe->on_empty)
+ return true;
+ if (jfe->on_error)
+ return true;
+ }

+ if (jfe->output && WALK(jfe->output))
+ return true;
can be simplified:

+ if (WALK(jfe->output))
+ return true;

view thread (24+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: remaining sql/json patches
  In-Reply-To: <CACJufxFkKV8-tMa88ByaabYDQan9+cFCcf1Z1jH7=bQuMAHN4A@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox