From: | jian he <jian(dot)universality(at)gmail(dot)com> |
---|---|
To: | Amit Langote <amitlangote09(at)gmail(dot)com> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Himanshu Upadhyaya <upadhyaya(dot)himanshu(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Erik Rijkers <er(at)xs4all(dot)nl>, Andres Freund <andres(at)anarazel(dot)de>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: remaining sql/json patches |
Date: | 2024-03-20 06:51:48 |
Message-ID: | CACJufxFkKV8-tMa88ByaabYDQan9+cFCcf1Z1jH7=bQuMAHN4A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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;
From | Date | Subject | |
---|---|---|---|
Next Message | John Naylor | 2024-03-20 06:57:54 | Re: add AVX2 support to simd.h |
Previous Message | John Naylor | 2024-03-20 06:48:10 | Re: [PoC] Improve dead tuple storage for lazy vacuum |