public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v45 1/3] Subscripting for jsonb
12+ messages / 7 participants
[nested] [flat]
* [PATCH v45 1/3] Subscripting for jsonb
@ 2020-12-18 16:19 Dmitrii Dolgov <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Dmitrii Dolgov @ 2020-12-18 16:19 UTC (permalink / raw)
Subscripting implementation for jsonb. It does not support slices, does
not have a limit for number of subscripts and for assignment expects a
replace value to be of jsonb type. There is also one functional
difference in assignment via subscripting from jsonb_set, when an
original jsonb container is NULL, subscripting replaces it with an empty
jsonb and proceed with assignment.
For the sake of code reuse, some parts of jsonb functionality were
rearranged to allow use the same functions for jsonb_set and assign
subscripting operation.
The original idea belongs to Oleg Bartunov.
Reviewed-by: Tom Lane, Arthur Zakirov, Pavel Stehule
---
doc/src/sgml/json.sgml | 48 ++++
src/backend/utils/adt/Makefile | 1 +
src/backend/utils/adt/jsonb_util.c | 76 ++++-
src/backend/utils/adt/jsonbsubs.c | 413 ++++++++++++++++++++++++++++
src/backend/utils/adt/jsonfuncs.c | 180 ++++++------
src/include/catalog/pg_proc.dat | 4 +
src/include/catalog/pg_type.dat | 3 +-
src/include/utils/jsonb.h | 6 +-
src/test/regress/expected/jsonb.out | 272 +++++++++++++++++-
src/test/regress/sql/jsonb.sql | 84 +++++-
10 files changed, 982 insertions(+), 105 deletions(-)
create mode 100644 src/backend/utils/adt/jsonbsubs.c
diff --git a/doc/src/sgml/json.sgml b/doc/src/sgml/json.sgml
index 5b9a5557a4..100d1a60f4 100644
--- a/doc/src/sgml/json.sgml
+++ b/doc/src/sgml/json.sgml
@@ -602,6 +602,54 @@ SELECT jdoc->'guid', jdoc->'name' FROM api WHERE jdoc @> '{"tags": ["qu
</para>
</sect2>
+ <sect2 id="jsonb-subscripting">
+ <title><type>jsonb</type> Subscripting</title>
+ <para>
+ <type>jsonb</type> data type supports array-style subscripting expressions
+ to extract or update particular elements. It's possible to use multiple
+ subscripting expressions to extract nested values. In this case, a chain of
+ subscripting expressions follows the same rules as the
+ <literal>path</literal> argument in <literal>jsonb_set</literal> function,
+ e.g. in case of arrays it is a 0-based operation or that negative integers
+ that appear in <literal>path</literal> count from the end of JSON arrays.
+ The result of subscripting expressions is always jsonb data type. An
+ example of subscripting syntax:
+<programlisting>
+-- Extract value by key
+SELECT ('{"a": 1}'::jsonb)['a'];
+
+-- Extract nested value by key path
+SELECT ('{"a": {"b": {"c": 1}}}'::jsonb)['a']['b']['c'];
+
+-- Extract element by index
+SELECT ('[1, "2", null]'::jsonb)[1];
+
+-- Update value by key, note the single quotes - the assigned value
+-- needs to be of jsonb type as well
+UPDATE table_name SET jsonb_field['key'] = '1';
+
+-- Select records using where clause with subscripting. Since the result of
+-- subscripting is jsonb and we basically want to compare two jsonb objects, we
+-- need to put the value in double quotes to be able to convert it to jsonb.
+SELECT * FROM table_name WHERE jsonb_field['key'] = '"value"';
+</programlisting>
+
+ Subscripting for <type>jsonb</type> does not support slice expressions,
+ even if it contains an array.
+
+ In case if source <type>jsonb</type> is <literal>NULL</literal>, assignment
+ via subscripting will proceed as if it was an empty JSON object:
+<programlisting>
+-- If jsonb_field here is NULL, the result is {"a": 1}
+UPDATE table_name SET jsonb_field['a'] = '1';
+
+-- If jsonb_field here is NULL, the result is [1]
+UPDATE table_name SET jsonb_field[0] = '1';
+</programlisting>
+
+ </para>
+ </sect2>
+
<sect2>
<title>Transforms</title>
diff --git a/src/backend/utils/adt/Makefile b/src/backend/utils/adt/Makefile
index 82732146d3..279ff15ade 100644
--- a/src/backend/utils/adt/Makefile
+++ b/src/backend/utils/adt/Makefile
@@ -50,6 +50,7 @@ OBJS = \
jsonb_op.o \
jsonb_util.o \
jsonfuncs.o \
+ jsonbsubs.o \
jsonpath.o \
jsonpath_exec.o \
jsonpath_gram.o \
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index 4eeffa1424..41a1c1f9bb 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -68,18 +68,29 @@ static JsonbValue *pushJsonbValueScalar(JsonbParseState **pstate,
JsonbIteratorToken seq,
JsonbValue *scalarVal);
+JsonbValue *
+JsonbToJsonbValue(Jsonb *jsonb)
+{
+ JsonbValue *val = (JsonbValue *) palloc(sizeof(JsonbValue));
+
+ val->type = jbvBinary;
+ val->val.binary.data = &jsonb->root;
+ val->val.binary.len = VARSIZE(jsonb) - VARHDRSZ;
+
+ return val;
+}
+
/*
* Turn an in-memory JsonbValue into a Jsonb for on-disk storage.
*
- * There isn't a JsonbToJsonbValue(), because generally we find it more
- * convenient to directly iterate through the Jsonb representation and only
- * really convert nested scalar values. JsonbIteratorNext() does this, so that
- * clients of the iteration code don't have to directly deal with the binary
- * representation (JsonbDeepContains() is a notable exception, although all
- * exceptions are internal to this module). In general, functions that accept
- * a JsonbValue argument are concerned with the manipulation of scalar values,
- * or simple containers of scalar values, where it would be inconvenient to
- * deal with a great amount of other state.
+ * Generally we find it more convenient to directly iterate through the Jsonb
+ * representation and only really convert nested scalar values.
+ * JsonbIteratorNext() does this, so that clients of the iteration code don't
+ * have to directly deal with the binary representation (JsonbDeepContains() is
+ * a notable exception, although all exceptions are internal to this module).
+ * In general, functions that accept a JsonbValue argument are concerned with
+ * the manipulation of scalar values, or simple containers of scalar values,
+ * where it would be inconvenient to deal with a great amount of other state.
*/
Jsonb *
JsonbValueToJsonb(JsonbValue *val)
@@ -563,6 +574,30 @@ pushJsonbValue(JsonbParseState **pstate, JsonbIteratorToken seq,
JsonbValue *res = NULL;
JsonbValue v;
JsonbIteratorToken tok;
+ int i;
+
+ if (jbval && (seq == WJB_ELEM || seq == WJB_VALUE) && jbval->type == jbvObject)
+ {
+ pushJsonbValue(pstate, WJB_BEGIN_OBJECT, NULL);
+ for (i = 0; i < jbval->val.object.nPairs; i++)
+ {
+ pushJsonbValue(pstate, WJB_KEY, &jbval->val.object.pairs[i].key);
+ pushJsonbValue(pstate, WJB_VALUE, &jbval->val.object.pairs[i].value);
+ }
+
+ return pushJsonbValue(pstate, WJB_END_OBJECT, NULL);
+ }
+
+ if (jbval && (seq == WJB_ELEM || seq == WJB_VALUE) && jbval->type == jbvArray)
+ {
+ pushJsonbValue(pstate, WJB_BEGIN_ARRAY, NULL);
+ for (i = 0; i < jbval->val.array.nElems; i++)
+ {
+ pushJsonbValue(pstate, WJB_ELEM, &jbval->val.array.elems[i]);
+ }
+
+ return pushJsonbValue(pstate, WJB_END_ARRAY, NULL);
+ }
if (!jbval || (seq != WJB_ELEM && seq != WJB_VALUE) ||
jbval->type != jbvBinary)
@@ -573,9 +608,30 @@ pushJsonbValue(JsonbParseState **pstate, JsonbIteratorToken seq,
/* unpack the binary and add each piece to the pstate */
it = JsonbIteratorInit(jbval->val.binary.data);
+
+ if ((jbval->val.binary.data->header & JB_FSCALAR) && *pstate)
+ {
+ tok = JsonbIteratorNext(&it, &v, true);
+ Assert(tok == WJB_BEGIN_ARRAY);
+ Assert(v.type == jbvArray && v.val.array.rawScalar);
+
+ tok = JsonbIteratorNext(&it, &v, true);
+ Assert(tok == WJB_ELEM);
+
+ res = pushJsonbValueScalar(pstate, seq, &v);
+
+ tok = JsonbIteratorNext(&it, &v, true);
+ Assert(tok == WJB_END_ARRAY);
+ Assert(it == NULL);
+
+ return res;
+ }
+
while ((tok = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
res = pushJsonbValueScalar(pstate, tok,
- tok < WJB_BEGIN_ARRAY ? &v : NULL);
+ tok < WJB_BEGIN_ARRAY ||
+ (tok == WJB_BEGIN_ARRAY &&
+ v.val.array.rawScalar) ? &v : NULL);
return res;
}
diff --git a/src/backend/utils/adt/jsonbsubs.c b/src/backend/utils/adt/jsonbsubs.c
new file mode 100644
index 0000000000..306c37b5a6
--- /dev/null
+++ b/src/backend/utils/adt/jsonbsubs.c
@@ -0,0 +1,413 @@
+/*-------------------------------------------------------------------------
+ *
+ * jsonbsubs.c
+ * Subscripting support functions for jsonb.
+ *
+ * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ * src/backend/utils/adt/jsonbsubs.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "executor/execExpr.h"
+#include "nodes/makefuncs.h"
+#include "nodes/nodeFuncs.h"
+#include "nodes/subscripting.h"
+#include "parser/parse_coerce.h"
+#include "parser/parse_expr.h"
+#include "utils/jsonb.h"
+#include "utils/jsonfuncs.h"
+#include "utils/builtins.h"
+#include "utils/lsyscache.h"
+
+
+/* SubscriptingRefState.workspace for jsonb subscripting execution */
+typedef struct JsonbSubWorkspace
+{
+ bool expectArray; /* jsonb root is expected to be an array */
+ Oid *indexOid; /* OID of coerced subscript expression,
+ could be only integer or text */
+ Datum *index; /* Subscript values in Datum format */
+} JsonbSubWorkspace;
+
+
+/*
+ * Finish parse analysis of a SubscriptingRef expression for a jsonb.
+ *
+ * Transform the subscript expressions, coerce them to text,
+ * and determine the result type of the SubscriptingRef node.
+ */
+static void
+jsonb_subscript_transform(SubscriptingRef *sbsref,
+ List *indirection,
+ ParseState *pstate,
+ bool isSlice,
+ bool isAssignment)
+{
+ List *upperIndexpr = NIL;
+ ListCell *idx;
+
+ /*
+ * Transform and convert the subscript expressions. Jsonb subscripting does
+ * not support slices, look only and the upper index.
+ */
+ foreach(idx, indirection)
+ {
+ A_Indices *ai = lfirst_node(A_Indices, idx);
+ Node *subExpr;
+
+ if (isSlice)
+ {
+ Node *expr = ai->uidx ? ai->uidx : ai->lidx;
+
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("jsonb subscript does not support slices"),
+ parser_errposition(pstate, exprLocation(expr))));
+ }
+
+ if (ai->uidx)
+ {
+ Oid subExprType = InvalidOid,
+ targetType = UNKNOWNOID;
+
+ subExpr = transformExpr(pstate, ai->uidx, pstate->p_expr_kind);
+ subExprType = exprType(subExpr);
+
+ if (subExprType != UNKNOWNOID)
+ {
+ Oid targets[2] = {INT4OID, TEXTOID};
+
+ /*
+ * Jsonb can handle multiple subscript types, but cases when a
+ * subscript could be coerced to multiple target types must be
+ * avoided, similar to overloaded functions. It could be
+ * possibly extend with jsonpath in the future.
+ */
+ for (int i = 0; i < 2; i++)
+ {
+ if (can_coerce_type(1, &subExprType, &targets[i], COERCION_IMPLICIT))
+ {
+ /*
+ * One type has already succeeded, it means there are
+ * two coercion targets possible, failure.
+ */
+ if (targetType != UNKNOWNOID)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("subscript type is not supported"),
+ errhint("Jsonb subscript must be coerced "
+ "only to one type, integer or text."),
+ parser_errposition(pstate, exprLocation(subExpr))));
+
+ targetType = targets[i];
+ }
+ }
+
+ /*
+ * No suitable types were found, failure.
+ */
+ if (targetType == UNKNOWNOID)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("subscript type is not supported"),
+ errhint("Jsonb subscript must be coercet to either integer or text"),
+ parser_errposition(pstate, exprLocation(subExpr))));
+ }
+ else
+ targetType = TEXTOID;
+
+ /*
+ * We known from can_coerce_type that coercion will succeed, so
+ * coerce_type could be used. Note the implicit coercion context,
+ * which is required to handle subscripts of different types,
+ * similar to overloaded functions.
+ */
+ subExpr = coerce_type(pstate,
+ subExpr, subExprType,
+ targetType, -1,
+ 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))));
+ }
+ else
+ {
+ /*
+ * Slice with omitted upper bound. Should not happen as we already
+ * errored out on slice earlier, but handle this just in case.
+ */
+ Assert(isSlice && ai->is_slice);
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("jsonb subscript does not support slices"),
+ parser_errposition(pstate, exprLocation(ai->uidx))));
+ }
+
+ upperIndexpr = lappend(upperIndexpr, subExpr);
+ }
+
+ /* store the transformed lists into the SubscriptRef node */
+ sbsref->refupperindexpr = upperIndexpr;
+ sbsref->reflowerindexpr = NIL;
+
+ /* Determine the result type of the subscripting operation; always jsonb */
+ sbsref->refrestype = JSONBOID;
+ sbsref->reftypmod = -1;
+}
+
+/*
+ * During execution, process the subscripts in a SubscriptingRef expression.
+ *
+ * The subscript expressions are already evaluated in Datum form in the
+ * SubscriptingRefState's arrays. Check and convert them as necessary.
+ *
+ * If any subscript is NULL, we throw error in assignment cases, or in fetch
+ * cases set result to NULL and return false (instructing caller to skip the
+ * rest of the SubscriptingRef sequence).
+ */
+static bool
+jsonb_subscript_check_subscripts(ExprState *state,
+ ExprEvalStep *op,
+ ExprContext *econtext)
+{
+ SubscriptingRefState *sbsrefstate = op->d.sbsref_subscript.state;
+ JsonbSubWorkspace *workspace = (JsonbSubWorkspace *) sbsrefstate->workspace;
+
+ /*
+ * In case if the first subscript is an integer, the source jsonb is
+ * expected to be an array. This information is not used directly, all such
+ * cases are handled within corresponding jsonb assign functions. But if
+ * the source jsonb is NULL the expected type will be used to construct an
+ * empty source.
+ */
+ if (sbsrefstate->numupper > 0 && sbsrefstate->upperprovided[0] &&
+ !sbsrefstate->upperindexnull[0] && workspace->indexOid[0] == INT4OID)
+ workspace->expectArray = true;
+
+ /* Process upper subscripts */
+ for (int i = 0; i < sbsrefstate->numupper; i++)
+ {
+ if (sbsrefstate->upperprovided[i])
+ {
+ /* If any index expr yields NULL, result is NULL or error */
+ if (sbsrefstate->upperindexnull[i])
+ {
+ if (sbsrefstate->isassignment)
+ ereport(ERROR,
+ (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+ errmsg("jsonb subscript in assignment must not be null")));
+ *op->resnull = true;
+ return false;
+ }
+
+ /*
+ * For jsonb fetch and assign functions we need to provide path in
+ * text format. Convert if it's not already text.
+ */
+ if (workspace->indexOid[i] == INT4OID)
+ {
+ Datum datum = sbsrefstate->upperindex[i];
+ char *cs = DatumGetCString(DirectFunctionCall1(int4out, datum));
+ workspace->index[i] = CStringGetTextDatum(cs);
+ }
+ else
+ workspace->index[i] = sbsrefstate->upperindex[i];
+ }
+ }
+
+ return true;
+}
+
+/*
+ * Evaluate SubscriptingRef fetch for a jsonb element.
+ *
+ * Source container is in step's result variable (it's known not NULL, since
+ * we set fetch_strict to true).
+ */
+static void
+jsonb_subscript_fetch(ExprState *state,
+ ExprEvalStep *op,
+ ExprContext *econtext)
+{
+ SubscriptingRefState *sbsrefstate = op->d.sbsref.state;
+ JsonbSubWorkspace *workspace = (JsonbSubWorkspace *) sbsrefstate->workspace;
+ Jsonb *jsonbSource;
+
+ /* Should not get here if source jsonb (or any subscript) is null */
+ Assert(!(*op->resnull));
+
+ jsonbSource = DatumGetJsonbP(*op->resvalue);
+ *op->resvalue = jsonb_get_element(jsonbSource,
+ workspace->index,
+ sbsrefstate->numupper,
+ op->resnull,
+ false);
+}
+
+/*
+ * Evaluate SubscriptingRef assignment for a jsonb element assignment.
+ *
+ * Input container (possibly null) is in result area, replacement value is in
+ * SubscriptingRefState's replacevalue/replacenull.
+ */
+static void
+jsonb_subscript_assign(ExprState *state,
+ ExprEvalStep *op,
+ ExprContext *econtext)
+{
+ SubscriptingRefState *sbsrefstate = op->d.sbsref.state;
+ JsonbSubWorkspace *workspace = (JsonbSubWorkspace *) sbsrefstate->workspace;
+ Jsonb *jsonbSource;
+ JsonbValue *replacevalue;
+
+ if (sbsrefstate->replacenull)
+ {
+ replacevalue = (JsonbValue *) palloc(sizeof(JsonbValue));
+ replacevalue->type = jbvNull;
+ }
+ else
+ replacevalue =
+ JsonbToJsonbValue(DatumGetJsonbP(sbsrefstate->replacevalue));
+
+ /*
+ * In case if the input container is null, set up an empty jsonb and
+ * proceed with the assignment.
+ */
+ if (*op->resnull)
+ {
+ JsonbValue *newSource = (JsonbValue *) palloc(sizeof(JsonbValue));
+
+ /*
+ * To avoid any surprising results, set up an empty jsonb array in case
+ * of an array is expected (i.e. the first subscript is integer),
+ * otherwise jsonb object.
+ */
+ if (workspace->expectArray)
+ {
+ newSource->type = jbvArray;
+ newSource->val.array.nElems = 0;
+ newSource->val.array.rawScalar = false;
+ }
+ else
+ {
+ newSource->type = jbvObject;
+ newSource->val.object.nPairs = 0;
+ }
+
+ jsonbSource = JsonbValueToJsonb(newSource);
+ *op->resnull = false;
+ }
+ else
+ jsonbSource = DatumGetJsonbP(*op->resvalue);
+
+ *op->resvalue = jsonb_set_element(jsonbSource,
+ workspace->index,
+ sbsrefstate->numupper,
+ replacevalue);
+ /* The result is never NULL, so no need to change *op->resnull */
+}
+
+/*
+ * Compute old jsonb element value for a SubscriptingRef assignment
+ * expression. Will only be called if the new-value subexpression
+ * contains SubscriptingRef or FieldStore. This is the same as the
+ * regular fetch case, except that we have to handle a null jsonb,
+ * and the value should be stored into the SubscriptingRefState's
+ * prevvalue/prevnull fields.
+ */
+static void
+jsonb_subscript_fetch_old(ExprState *state,
+ ExprEvalStep *op,
+ ExprContext *econtext)
+{
+ SubscriptingRefState *sbsrefstate = op->d.sbsref.state;
+
+ if (*op->resnull)
+ {
+ /* whole jsonb is null, so any element is too */
+ sbsrefstate->prevvalue = (Datum) 0;
+ sbsrefstate->prevnull = true;
+ }
+ else
+ {
+ Jsonb *jsonbSource = DatumGetJsonbP(*op->resvalue);
+ sbsrefstate->prevvalue = jsonb_get_element(jsonbSource,
+ sbsrefstate->upperindex,
+ sbsrefstate->numupper,
+ &sbsrefstate->prevnull,
+ false);
+ }
+}
+
+/*
+ * Set up execution state for a jsonb subscript operation. Opposite to the
+ * arrays subscription, there is no limit for number of subscripts as jsonb
+ * type itself doesn't have nesting limits.
+ */
+static void
+jsonb_exec_setup(const SubscriptingRef *sbsref,
+ SubscriptingRefState *sbsrefstate,
+ SubscriptExecSteps *methods)
+{
+ JsonbSubWorkspace *workspace;
+ ListCell *lc;
+ int nupper = sbsref->refupperindexpr->length;
+ char *ptr;
+
+ /* Allocate type-specific workspace with space for per-subscript data */
+ workspace = palloc0(MAXALIGN(sizeof(JsonbSubWorkspace)) +
+ nupper * (sizeof(Datum) + sizeof(Oid)));
+ workspace->expectArray = false;
+ ptr = ((char *) workspace) + MAXALIGN(sizeof(JsonbSubWorkspace));
+ workspace->indexOid = (Oid *) ptr;
+ ptr += nupper * sizeof(Oid);
+ workspace->index = (Datum *) ptr;
+
+ sbsrefstate->workspace = workspace;
+
+ /* Collect subscript data types necessary at execution time */
+ foreach(lc, sbsref->refupperindexpr)
+ {
+ Node *expr = lfirst(lc);
+ int i = foreach_current_index(lc);
+
+ workspace->indexOid[i] = exprType(expr);
+ }
+
+ /*
+ * Pass back pointers to appropriate step execution functions.
+ */
+ methods->sbs_check_subscripts = jsonb_subscript_check_subscripts;
+ methods->sbs_fetch = jsonb_subscript_fetch;
+ methods->sbs_assign = jsonb_subscript_assign;
+ methods->sbs_fetch_old = jsonb_subscript_fetch_old;
+}
+
+/*
+ * jsonb_subscript_handler
+ * Subscripting handler for jsonb.
+ *
+ */
+Datum
+jsonb_subscript_handler(PG_FUNCTION_ARGS)
+{
+ static const SubscriptRoutines sbsroutines = {
+ .transform = jsonb_subscript_transform,
+ .exec_setup = jsonb_exec_setup,
+ .fetch_strict = true, /* fetch returns NULL for NULL inputs */
+ .fetch_leakproof = true, /* fetch returns NULL for bad subscript */
+ .store_leakproof = false /* ... but assignment throws error */
+ };
+
+ PG_RETURN_POINTER(&sbsroutines);
+}
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 69100feab7..5a0ba6b220 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -461,18 +461,18 @@ static Datum populate_domain(DomainIOData *io, Oid typid, const char *colname,
/* functions supporting jsonb_delete, jsonb_set and jsonb_concat */
static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
JsonbParseState **state);
-static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
+extern JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
bool *path_nulls, int path_len,
- JsonbParseState **st, int level, Jsonb *newval,
+ JsonbParseState **st, int level, JsonbValue *newval,
int op_type);
static void setPathObject(JsonbIterator **it, Datum *path_elems,
bool *path_nulls, int path_len, JsonbParseState **st,
int level,
- Jsonb *newval, uint32 npairs, int op_type);
+ JsonbValue *newval, uint32 npairs, int op_type);
static void setPathArray(JsonbIterator **it, Datum *path_elems,
bool *path_nulls, int path_len, JsonbParseState **st,
- int level, Jsonb *newval, uint32 nelems, int op_type);
-static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
+ int level,
+ JsonbValue *newval, uint32 nelems, int op_type);
/* function supporting iterate_json_values */
static void iterate_values_scalar(void *state, char *token, JsonTokenType tokentype);
@@ -1448,13 +1448,9 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
ArrayType *path = PG_GETARG_ARRAYTYPE_P(1);
Datum *pathtext;
bool *pathnulls;
+ bool isnull;
int npath;
- int i;
- bool have_object = false,
- have_array = false;
- JsonbValue *jbvp = NULL;
- JsonbValue jbvbuf;
- JsonbContainer *container;
+ Datum res;
/*
* If the array contains any null elements, return NULL, on the grounds
@@ -1469,9 +1465,26 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
deconstruct_array(path, TEXTOID, -1, false, TYPALIGN_INT,
&pathtext, &pathnulls, &npath);
- /* Identify whether we have object, array, or scalar at top-level */
- container = &jb->root;
+ res = jsonb_get_element(jb, pathtext, npath, &isnull, as_text);
+ if (isnull)
+ PG_RETURN_NULL();
+ else
+ PG_RETURN_DATUM(res);
+}
+
+Datum
+jsonb_get_element(Jsonb *jb, Datum *path, int npath, bool *isnull, bool as_text)
+{
+ JsonbContainer *container = &jb->root;
+ JsonbValue *jbvp = NULL;
+ int i;
+ bool have_object = false,
+ have_array = false;
+
+ *isnull = false;
+
+ /* Identify whether we have object, array, or scalar at top-level */
if (JB_ROOT_IS_OBJECT(jb))
have_object = true;
else if (JB_ROOT_IS_ARRAY(jb) && !JB_ROOT_IS_SCALAR(jb))
@@ -1496,7 +1509,7 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
{
if (as_text)
{
- PG_RETURN_TEXT_P(cstring_to_text(JsonbToCString(NULL,
+ return PointerGetDatum(cstring_to_text(JsonbToCString(NULL,
container,
VARSIZE(jb))));
}
@@ -1512,22 +1525,25 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
if (have_object)
{
jbvp = getKeyJsonValueFromContainer(container,
- VARDATA(pathtext[i]),
- VARSIZE(pathtext[i]) - VARHDRSZ,
- &jbvbuf);
+ VARDATA(path[i]),
+ VARSIZE(path[i]) - VARHDRSZ,
+ NULL);
}
else if (have_array)
{
long lindex;
uint32 index;
- char *indextext = TextDatumGetCString(pathtext[i]);
+ char *indextext = TextDatumGetCString(path[i]);
char *endptr;
errno = 0;
lindex = strtol(indextext, &endptr, 10);
if (endptr == indextext || *endptr != '\0' || errno != 0 ||
lindex > INT_MAX || lindex < INT_MIN)
- PG_RETURN_NULL();
+ {
+ *isnull = true;
+ return PointerGetDatum(NULL);
+ }
if (lindex >= 0)
{
@@ -1545,7 +1561,10 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
nelements = JsonContainerSize(container);
if (-lindex > nelements)
- PG_RETURN_NULL();
+ {
+ *isnull = true;
+ return PointerGetDatum(NULL);
+ }
else
index = nelements + lindex;
}
@@ -1555,11 +1574,15 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
else
{
/* scalar, extraction yields a null */
- PG_RETURN_NULL();
+ *isnull = true;
+ return PointerGetDatum(NULL);
}
if (jbvp == NULL)
- PG_RETURN_NULL();
+ {
+ *isnull = true;
+ return PointerGetDatum(NULL);
+ }
else if (i == npath - 1)
break;
@@ -1581,9 +1604,12 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
if (as_text)
{
if (jbvp->type == jbvNull)
- PG_RETURN_NULL();
+ {
+ *isnull = true;
+ return PointerGetDatum(NULL);
+ }
- PG_RETURN_TEXT_P(JsonbValueAsText(jbvp));
+ return PointerGetDatum(JsonbValueAsText(jbvp));
}
else
{
@@ -1594,6 +1620,28 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
}
}
+Datum
+jsonb_set_element(Jsonb* jb, Datum *path, int path_len,
+ JsonbValue *newval)
+{
+ JsonbValue *res;
+ JsonbParseState *state = NULL;
+ JsonbIterator *it;
+ bool *path_nulls = palloc0(path_len * sizeof(bool));
+
+ if (newval->type == jbvArray && newval->val.array.rawScalar)
+ *newval = newval->val.array.elems[0];
+
+ it = JsonbIteratorInit(&jb->root);
+
+ res = setPath(&it, path, path_nulls, path_len, &state, 0,
+ newval, JB_PATH_CREATE);
+
+ pfree(path_nulls);
+
+ PG_RETURN_JSONB_P(JsonbValueToJsonb(res));
+}
+
/*
* Return the text representation of the given JsonbValue.
*/
@@ -4151,58 +4199,6 @@ jsonb_strip_nulls(PG_FUNCTION_ARGS)
PG_RETURN_POINTER(JsonbValueToJsonb(res));
}
-/*
- * Add values from the jsonb to the parse state.
- *
- * If the parse state container is an object, the jsonb is pushed as
- * a value, not a key.
- *
- * This needs to be done using an iterator because pushJsonbValue doesn't
- * like getting jbvBinary values, so we can't just push jb as a whole.
- */
-static void
-addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb)
-{
- JsonbIterator *it;
- JsonbValue *o = &(*jbps)->contVal;
- JsonbValue v;
- JsonbIteratorToken type;
-
- it = JsonbIteratorInit(&jb->root);
-
- Assert(o->type == jbvArray || o->type == jbvObject);
-
- if (JB_ROOT_IS_SCALAR(jb))
- {
- (void) JsonbIteratorNext(&it, &v, false); /* skip array header */
- Assert(v.type == jbvArray);
- (void) JsonbIteratorNext(&it, &v, false); /* fetch scalar value */
-
- switch (o->type)
- {
- case jbvArray:
- (void) pushJsonbValue(jbps, WJB_ELEM, &v);
- break;
- case jbvObject:
- (void) pushJsonbValue(jbps, WJB_VALUE, &v);
- break;
- default:
- elog(ERROR, "unexpected parent of nested structure");
- }
- }
- else
- {
- while ((type = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
- {
- if (type == WJB_KEY || type == WJB_VALUE || type == WJB_ELEM)
- (void) pushJsonbValue(jbps, type, &v);
- else
- (void) pushJsonbValue(jbps, type, NULL);
- }
- }
-
-}
-
/*
* SQL function jsonb_pretty (jsonb)
*
@@ -4474,7 +4470,8 @@ jsonb_set(PG_FUNCTION_ARGS)
{
Jsonb *in = PG_GETARG_JSONB_P(0);
ArrayType *path = PG_GETARG_ARRAYTYPE_P(1);
- Jsonb *newval = PG_GETARG_JSONB_P(2);
+ Jsonb *newjsonb = PG_GETARG_JSONB_P(2);
+ JsonbValue *newval = JsonbToJsonbValue(newjsonb);
bool create = PG_GETARG_BOOL(3);
JsonbValue *res = NULL;
Datum *path_elems;
@@ -4632,7 +4629,8 @@ jsonb_insert(PG_FUNCTION_ARGS)
{
Jsonb *in = PG_GETARG_JSONB_P(0);
ArrayType *path = PG_GETARG_ARRAYTYPE_P(1);
- Jsonb *newval = PG_GETARG_JSONB_P(2);
+ Jsonb *newjsonb = PG_GETARG_JSONB_P(2);
+ JsonbValue *newval = JsonbToJsonbValue(newjsonb);
bool after = PG_GETARG_BOOL(3);
JsonbValue *res = NULL;
Datum *path_elems;
@@ -4787,10 +4785,10 @@ IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
* All path elements before the last must already exist
* whatever bits in op_type are set, or nothing is done.
*/
-static JsonbValue *
+JsonbValue *
setPath(JsonbIterator **it, Datum *path_elems,
bool *path_nulls, int path_len,
- JsonbParseState **st, int level, Jsonb *newval, int op_type)
+ JsonbParseState **st, int level, JsonbValue *newval, int op_type)
{
JsonbValue v;
JsonbIteratorToken r;
@@ -4843,11 +4841,11 @@ setPath(JsonbIterator **it, Datum *path_elems,
static void
setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
int path_len, JsonbParseState **st, int level,
- Jsonb *newval, uint32 npairs, int op_type)
+ JsonbValue *newval, uint32 npairs, int op_type)
{
- JsonbValue v;
int i;
- JsonbValue k;
+ JsonbValue k,
+ v;
bool done = false;
if (level >= path_len || path_nulls[level])
@@ -4864,7 +4862,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
newkey.val.string.val = VARDATA_ANY(path_elems[level]);
(void) pushJsonbValue(st, WJB_KEY, &newkey);
- addJsonbToParseState(st, newval);
+ (void) pushJsonbValue(st, WJB_VALUE, newval);
}
for (i = 0; i < npairs; i++)
@@ -4895,7 +4893,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
if (!(op_type & JB_PATH_DELETE))
{
(void) pushJsonbValue(st, WJB_KEY, &k);
- addJsonbToParseState(st, newval);
+ (void) pushJsonbValue(st, WJB_VALUE, newval);
}
done = true;
}
@@ -4918,7 +4916,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
newkey.val.string.val = VARDATA_ANY(path_elems[level]);
(void) pushJsonbValue(st, WJB_KEY, &newkey);
- addJsonbToParseState(st, newval);
+ (void) pushJsonbValue(st, WJB_VALUE, newval);
}
(void) pushJsonbValue(st, r, &k);
@@ -4950,7 +4948,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
static void
setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
int path_len, JsonbParseState **st, int level,
- Jsonb *newval, uint32 nelems, int op_type)
+ JsonbValue *newval, uint32 nelems, int op_type)
{
JsonbValue v;
int idx,
@@ -4998,7 +4996,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
(op_type & JB_PATH_CREATE_OR_INSERT))
{
Assert(newval != NULL);
- addJsonbToParseState(st, newval);
+ (void) pushJsonbValue(st, WJB_ELEM, newval);
done = true;
}
@@ -5014,7 +5012,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
r = JsonbIteratorNext(it, &v, true); /* skip */
if (op_type & (JB_PATH_INSERT_BEFORE | JB_PATH_CREATE))
- addJsonbToParseState(st, newval);
+ (void) pushJsonbValue(st, WJB_ELEM, newval);
/*
* We should keep current value only in case of
@@ -5025,7 +5023,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
(void) pushJsonbValue(st, r, &v);
if (op_type & (JB_PATH_INSERT_AFTER | JB_PATH_REPLACE))
- addJsonbToParseState(st, newval);
+ (void) pushJsonbValue(st, WJB_ELEM, newval);
done = true;
}
@@ -5059,7 +5057,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
if ((op_type & JB_PATH_CREATE_OR_INSERT) && !done &&
level == path_len - 1 && i == nelems - 1)
{
- addJsonbToParseState(st, newval);
+ (void) pushJsonbValue(st, WJB_ELEM, newval);
}
}
}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 139f4a08bd..feae8cc4b0 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -11239,6 +11239,10 @@
{ oid => '9256', descr => 'raw array subscripting support',
proname => 'raw_array_subscript_handler', prorettype => 'internal',
proargtypes => 'internal', prosrc => 'raw_array_subscript_handler' },
+# type subscripting support
+{ oid => '6098', descr => 'jsonb subscripting logic',
+ proname => 'jsonb_subscript_handler', prorettype => 'internal',
+ proargtypes => 'internal', prosrc => 'jsonb_subscript_handler' },
# collation management functions
{ oid => '3445', descr => 'import collations from operating system',
diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat
index 62018f063a..4a530ca907 100644
--- a/src/include/catalog/pg_type.dat
+++ b/src/include/catalog/pg_type.dat
@@ -444,7 +444,8 @@
{ oid => '3802', array_type_oid => '3807', descr => 'Binary JSON',
typname => 'jsonb', typlen => '-1', typbyval => 'f', typcategory => 'U',
typinput => 'jsonb_in', typoutput => 'jsonb_out', typreceive => 'jsonb_recv',
- typsend => 'jsonb_send', typalign => 'i', typstorage => 'x' },
+ typsend => 'jsonb_send', typalign => 'i', typstorage => 'x',
+ typsubscript => 'jsonb_subscript_handler' },
{ oid => '4072', array_type_oid => '4073', descr => 'JSON path',
typname => 'jsonpath', typlen => '-1', typbyval => 'f', typcategory => 'U',
typinput => 'jsonpath_in', typoutput => 'jsonpath_out',
diff --git a/src/include/utils/jsonb.h b/src/include/utils/jsonb.h
index 5860011693..64f1ccbf77 100644
--- a/src/include/utils/jsonb.h
+++ b/src/include/utils/jsonb.h
@@ -392,6 +392,7 @@ extern JsonbValue *pushJsonbValue(JsonbParseState **pstate,
extern JsonbIterator *JsonbIteratorInit(JsonbContainer *container);
extern JsonbIteratorToken JsonbIteratorNext(JsonbIterator **it, JsonbValue *val,
bool skipNested);
+extern JsonbValue *JsonbToJsonbValue(Jsonb *jsonb);
extern Jsonb *JsonbValueToJsonb(JsonbValue *val);
extern bool JsonbDeepContains(JsonbIterator **val,
JsonbIterator **mContained);
@@ -407,5 +408,8 @@ extern char *JsonbToCStringIndent(StringInfo out, JsonbContainer *in,
extern bool JsonbExtractScalar(JsonbContainer *jbc, JsonbValue *res);
extern const char *JsonbTypeName(JsonbValue *jb);
-
+extern Datum jsonb_set_element(Jsonb *jb, Datum *path, int path_len,
+ JsonbValue *newval);
+extern Datum jsonb_get_element(Jsonb *jb, Datum *path, int npath,
+ bool *isnull, bool as_text);
#endif /* __JSONB_H__ */
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 1e6c6ef200..bb3f25ec3f 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -4599,7 +4599,7 @@ select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'use_
{"a": 1, "b": null}
(1 row)
-\pset null
+\pset null ''
-- jsonb_insert
select jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"');
jsonb_insert
@@ -4729,6 +4729,276 @@ HINT: Try using the function jsonb_set to replace key value.
select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"', true);
ERROR: cannot replace existing key
HINT: Try using the function jsonb_set to replace key value.
+-- jsonb subscript
+select ('123'::jsonb)['a'];
+ jsonb
+-------
+
+(1 row)
+
+select ('123'::jsonb)[0];
+ jsonb
+-------
+
+(1 row)
+
+select ('123'::jsonb)[NULL];
+ jsonb
+-------
+
+(1 row)
+
+select ('{"a": 1}'::jsonb)['a'];
+ jsonb
+-------
+ 1
+(1 row)
+
+select ('{"a": 1}'::jsonb)[0];
+ jsonb
+-------
+
+(1 row)
+
+select ('{"a": 1}'::jsonb)['not_exist'];
+ jsonb
+-------
+
+(1 row)
+
+select ('{"a": 1}'::jsonb)[NULL];
+ jsonb
+-------
+
+(1 row)
+
+select ('[1, "2", null]'::jsonb)['a'];
+ jsonb
+-------
+
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[0];
+ jsonb
+-------
+ 1
+(1 row)
+
+select ('[1, "2", null]'::jsonb)['1'];
+ jsonb
+-------
+ "2"
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[1.0];
+ERROR: subscript type is not supported
+LINE 1: select ('[1, "2", null]'::jsonb)[1.0];
+ ^
+HINT: Jsonb subscript must be coercet to either integer or text
+select ('[1, "2", null]'::jsonb)[2];
+ jsonb
+-------
+ null
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[3];
+ jsonb
+-------
+
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[-2];
+ jsonb
+-------
+ "2"
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[1]['a'];
+ jsonb
+-------
+
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[1][0];
+ jsonb
+-------
+
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['b'];
+ jsonb
+-------
+ "c"
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'];
+ jsonb
+-----------
+ [1, 2, 3]
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'][1];
+ jsonb
+-------
+ 2
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d']['a'];
+ jsonb
+-------
+
+(1 row)
+
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1'];
+ jsonb
+---------------
+ {"a2": "aaa"}
+(1 row)
+
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2'];
+ jsonb
+-------
+ "aaa"
+(1 row)
+
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2']['a3'];
+ jsonb
+-------
+
+(1 row)
+
+select ('{"a": ["a1", {"b1": ["aaa", "bbb", "ccc"]}], "b": "bb"}'::jsonb)['a'][1]['b1'];
+ jsonb
+-----------------------
+ ["aaa", "bbb", "ccc"]
+(1 row)
+
+select ('{"a": ["a1", {"b1": ["aaa", "bbb", "ccc"]}], "b": "bb"}'::jsonb)['a'][1]['b1'][2];
+ jsonb
+-------
+ "ccc"
+(1 row)
+
+-- slices are not supported
+select ('{"a": 1}'::jsonb)['a':'b'];
+ERROR: jsonb subscript does not support slices
+LINE 1: select ('{"a": 1}'::jsonb)['a':'b'];
+ ^
+select ('[1, "2", null]'::jsonb)[1:2];
+ERROR: jsonb subscript does not support slices
+LINE 1: select ('[1, "2", null]'::jsonb)[1:2];
+ ^
+select ('[1, "2", null]'::jsonb)[:2];
+ERROR: jsonb subscript does not support slices
+LINE 1: select ('[1, "2", null]'::jsonb)[:2];
+ ^
+select ('[1, "2", null]'::jsonb)[1:];
+ERROR: jsonb subscript does not support slices
+LINE 1: select ('[1, "2", null]'::jsonb)[1:];
+ ^
+select ('[1, "2", null]'::jsonb)[:];
+ERROR: jsonb subscript does not support slices
+create TEMP TABLE test_jsonb_subscript (
+ id int,
+ test_json jsonb
+);
+insert into test_jsonb_subscript values
+(1, '{}'), -- empty jsonb
+(2, '{"key": "value"}'); -- jsonb with data
+-- update empty jsonb
+update test_jsonb_subscript set test_json['a'] = '1' where id = 1;
+select * from test_jsonb_subscript;
+ id | test_json
+----+------------------
+ 2 | {"key": "value"}
+ 1 | {"a": 1}
+(2 rows)
+
+-- update jsonb with some data
+update test_jsonb_subscript set test_json['a'] = '1' where id = 2;
+select * from test_jsonb_subscript;
+ id | test_json
+----+--------------------------
+ 1 | {"a": 1}
+ 2 | {"a": 1, "key": "value"}
+(2 rows)
+
+-- replace jsonb
+update test_jsonb_subscript set test_json['a'] = '"test"';
+select * from test_jsonb_subscript;
+ id | test_json
+----+-------------------------------
+ 1 | {"a": "test"}
+ 2 | {"a": "test", "key": "value"}
+(2 rows)
+
+-- replace by object
+update test_jsonb_subscript set test_json['a'] = '{"b": 1}'::jsonb;
+select * from test_jsonb_subscript;
+ id | test_json
+----+---------------------------------
+ 1 | {"a": {"b": 1}}
+ 2 | {"a": {"b": 1}, "key": "value"}
+(2 rows)
+
+-- replace by array
+update test_jsonb_subscript set test_json['a'] = '[1, 2, 3]'::jsonb;
+select * from test_jsonb_subscript;
+ id | test_json
+----+----------------------------------
+ 1 | {"a": [1, 2, 3]}
+ 2 | {"a": [1, 2, 3], "key": "value"}
+(2 rows)
+
+-- use jsonb subscription in where clause
+select * from test_jsonb_subscript where test_json['key'] = '"value"';
+ id | test_json
+----+----------------------------------
+ 2 | {"a": [1, 2, 3], "key": "value"}
+(1 row)
+
+select * from test_jsonb_subscript where test_json['key_doesnt_exists'] = '"value"';
+ id | test_json
+----+-----------
+(0 rows)
+
+select * from test_jsonb_subscript where test_json['key'] = '"wrong_value"';
+ id | test_json
+----+-----------
+(0 rows)
+
+-- NULL
+update test_jsonb_subscript set test_json[NULL] = '1';
+ERROR: jsonb subscript in assignment must not be null
+update test_jsonb_subscript set test_json['another_key'] = NULL;
+select * from test_jsonb_subscript;
+ id | test_json
+----+-------------------------------------------------------
+ 1 | {"a": [1, 2, 3], "another_key": null}
+ 2 | {"a": [1, 2, 3], "key": "value", "another_key": null}
+(2 rows)
+
+-- NULL as jsonb source
+insert into test_jsonb_subscript values (3, NULL);
+update test_jsonb_subscript set test_json['a'] = '1' where id = 3;
+select * from test_jsonb_subscript;
+ id | test_json
+----+-------------------------------------------------------
+ 1 | {"a": [1, 2, 3], "another_key": null}
+ 2 | {"a": [1, 2, 3], "key": "value", "another_key": null}
+ 3 | {"a": 1}
+(3 rows)
+
+update test_jsonb_subscript set test_json = NULL where id = 3;
+update test_jsonb_subscript set test_json[0] = '1';
+select * from test_jsonb_subscript;
+ id | test_json
+----+---------------------------------------------------------------
+ 1 | {"0": 1, "a": [1, 2, 3], "another_key": null}
+ 2 | {"0": 1, "a": [1, 2, 3], "key": "value", "another_key": null}
+ 3 | [1]
+(3 rows)
+
-- jsonb to tsvector
select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb);
to_tsvector
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index b6409767f6..20aa8fe0e2 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -1177,7 +1177,7 @@ select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'retu
select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'delete_key') as delete_key;
select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'use_json_null') as use_json_null;
-\pset null
+\pset null ''
-- jsonb_insert
select jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"');
@@ -1208,6 +1208,88 @@ select jsonb_insert('{"a": {"b": "value"}}', '{a, c}', '"new_value"', true);
select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"');
select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"', true);
+-- jsonb subscript
+select ('123'::jsonb)['a'];
+select ('123'::jsonb)[0];
+select ('123'::jsonb)[NULL];
+select ('{"a": 1}'::jsonb)['a'];
+select ('{"a": 1}'::jsonb)[0];
+select ('{"a": 1}'::jsonb)['not_exist'];
+select ('{"a": 1}'::jsonb)[NULL];
+select ('[1, "2", null]'::jsonb)['a'];
+select ('[1, "2", null]'::jsonb)[0];
+select ('[1, "2", null]'::jsonb)['1'];
+select ('[1, "2", null]'::jsonb)[1.0];
+select ('[1, "2", null]'::jsonb)[2];
+select ('[1, "2", null]'::jsonb)[3];
+select ('[1, "2", null]'::jsonb)[-2];
+select ('[1, "2", null]'::jsonb)[1]['a'];
+select ('[1, "2", null]'::jsonb)[1][0];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['b'];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'][1];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d']['a'];
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1'];
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2'];
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2']['a3'];
+select ('{"a": ["a1", {"b1": ["aaa", "bbb", "ccc"]}], "b": "bb"}'::jsonb)['a'][1]['b1'];
+select ('{"a": ["a1", {"b1": ["aaa", "bbb", "ccc"]}], "b": "bb"}'::jsonb)['a'][1]['b1'][2];
+
+-- slices are not supported
+select ('{"a": 1}'::jsonb)['a':'b'];
+select ('[1, "2", null]'::jsonb)[1:2];
+select ('[1, "2", null]'::jsonb)[:2];
+select ('[1, "2", null]'::jsonb)[1:];
+select ('[1, "2", null]'::jsonb)[:];
+
+create TEMP TABLE test_jsonb_subscript (
+ id int,
+ test_json jsonb
+);
+
+insert into test_jsonb_subscript values
+(1, '{}'), -- empty jsonb
+(2, '{"key": "value"}'); -- jsonb with data
+
+-- update empty jsonb
+update test_jsonb_subscript set test_json['a'] = '1' where id = 1;
+select * from test_jsonb_subscript;
+
+-- update jsonb with some data
+update test_jsonb_subscript set test_json['a'] = '1' where id = 2;
+select * from test_jsonb_subscript;
+
+-- replace jsonb
+update test_jsonb_subscript set test_json['a'] = '"test"';
+select * from test_jsonb_subscript;
+
+-- replace by object
+update test_jsonb_subscript set test_json['a'] = '{"b": 1}'::jsonb;
+select * from test_jsonb_subscript;
+
+-- replace by array
+update test_jsonb_subscript set test_json['a'] = '[1, 2, 3]'::jsonb;
+select * from test_jsonb_subscript;
+
+-- use jsonb subscription in where clause
+select * from test_jsonb_subscript where test_json['key'] = '"value"';
+select * from test_jsonb_subscript where test_json['key_doesnt_exists'] = '"value"';
+select * from test_jsonb_subscript where test_json['key'] = '"wrong_value"';
+
+-- NULL
+update test_jsonb_subscript set test_json[NULL] = '1';
+update test_jsonb_subscript set test_json['another_key'] = NULL;
+select * from test_jsonb_subscript;
+
+-- NULL as jsonb source
+insert into test_jsonb_subscript values (3, NULL);
+update test_jsonb_subscript set test_json['a'] = '1' where id = 3;
+select * from test_jsonb_subscript;
+
+update test_jsonb_subscript set test_json = NULL where id = 3;
+update test_jsonb_subscript set test_json[0] = '1';
+select * from test_jsonb_subscript;
+
-- jsonb to tsvector
select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb);
--
2.21.0
--2v7pubbjx3p2om6u
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v45-0002-Filling-gaps-in-jsonb.patch"
^ permalink raw reply [nested|flat] 12+ messages in thread
* pgsql: Add function to log the memory contexts of specified backend pro
@ 2021-04-06 04:45 Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Fujii Masao @ 2021-04-06 04:45 UTC (permalink / raw)
To: [email protected]
Add function to log the memory contexts of specified backend process.
Commit 3e98c0bafb added pg_backend_memory_contexts view to display
the memory contexts of the backend process. However its target process
is limited to the backend that is accessing to the view. So this is
not so convenient when investigating the local memory bloat of other
backend process. To improve this situation, this commit adds
pg_log_backend_memory_contexts() function that requests to log
the memory contexts of the specified backend process.
This information can be also collected by calling
MemoryContextStats(TopMemoryContext) via a debugger. But
this technique cannot be used in some environments because no debugger
is available there. So, pg_log_backend_memory_contexts() allows us to
see the memory contexts of specified backend more easily.
Only superusers are allowed to request to log the memory contexts
because allowing any users to issue this request at an unbounded rate
would cause lots of log messages and which can lead to denial of service.
On receipt of the request, at the next CHECK_FOR_INTERRUPTS(),
the target backend logs its memory contexts at LOG_SERVER_ONLY level,
so that these memory contexts will appear in the server log but not
be sent to the client. It logs one message per memory context.
Because if it buffers all memory contexts into StringInfo to log them
as one message, which may require the buffer to be enlarged very much
and lead to OOM error since there can be a large number of memory
contexts in a backend.
When a backend process is consuming huge memory, logging all its
memory contexts might overrun available disk space. To prevent this,
now this patch limits the number of child contexts to log per parent
to 100. As with MemoryContextStats(), it supposes that practical cases
where the log gets long will typically be huge numbers of siblings
under the same parent context; while the additional debugging value
from seeing details about individual siblings beyond 100 will not be large.
There was another proposed patch to add the function to return
the memory contexts of specified backend as the result sets,
instead of logging them, in the discussion. However that patch is
not included in this commit because it had several issues to address.
Thanks to Tatsuhito Kasahara, Andres Freund, Tom Lane, Tomas Vondra,
Michael Paquier, Kyotaro Horiguchi and Zhihong Yu for the discussion.
Bump catalog version.
Author: Atsushi Torikoshi
Reviewed-by: Kyotaro Horiguchi, Zhihong Yu, Fujii Masao
Discussion: https://postgr.es/m/[email protected]
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/43620e328617c1f41a2a54c8cee01723064e3ffa
Modified Files
--------------
doc/src/sgml/func.sgml | 52 ++++++++
src/backend/storage/ipc/procsignal.c | 4 +
src/backend/tcop/postgres.c | 3 +
src/backend/utils/adt/mcxtfuncs.c | 60 ++++++++-
src/backend/utils/init/globals.c | 1 +
src/backend/utils/mmgr/aset.c | 8 +-
src/backend/utils/mmgr/generation.c | 8 +-
src/backend/utils/mmgr/mcxt.c | 180 ++++++++++++++++++++++-----
src/backend/utils/mmgr/slab.c | 9 +-
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_proc.dat | 6 +
src/include/miscadmin.h | 1 +
src/include/nodes/memnodes.h | 6 +-
src/include/storage/procsignal.h | 1 +
src/include/utils/memutils.h | 5 +-
src/test/regress/expected/misc_functions.out | 13 ++
src/test/regress/sql/misc_functions.sql | 9 ++
17 files changed, 320 insertions(+), 48 deletions(-)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
@ 2025-04-30 17:15 ` Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Robert Haas @ 2025-04-30 17:15 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Tue, Apr 6, 2021 at 12:45 AM Fujii Masao <[email protected]> wrote:
> Add function to log the memory contexts of specified backend process.
Hi,
I think this might need a recursion guard. I tried this:
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index dc4c600922d..b219a934034 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3532,7 +3532,7 @@ ProcessInterrupts(void)
if (ParallelMessagePending)
ProcessParallelMessages();
- if (LogMemoryContextPending)
+ if (true)
ProcessLogMemoryContextInterrupt();
if (PublishMemoryContextPending)
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 72f5655fb34..867fd7b0ad5 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -112,7 +112,7 @@ extern void ProcessInterrupts(void);
/* Test whether an interrupt is pending */
#ifndef WIN32
#define INTERRUPTS_PENDING_CONDITION() \
- (unlikely(InterruptPending))
+ (unlikely(InterruptPending) || true)
#else
#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
pgwin32_dispatch_queued_signals() : 0, \
That immediately caused infinite recursion, ending in a core dump:
frame #13: 0x0000000104607b00
postgres`errfinish(filename=<unavailable>, lineno=<unavailable>,
funcname=<unavailable>) at elog.c:543:2 [opt]
frame #14: 0x0000000104637078
postgres`ProcessLogMemoryContextInterrupt at mcxt.c:1392:2 [opt]
frame #15: 0x00000001044a901c postgres`ProcessInterrupts at
postgres.c:3536:3 [opt]
frame #16: 0x0000000104607b54
postgres`errfinish(filename=<unavailable>, lineno=<unavailable>,
funcname=<unavailable>) at elog.c:608:2 [opt] [artificial]
frame #17: 0x0000000104637078
postgres`ProcessLogMemoryContextInterrupt at mcxt.c:1392:2 [opt]
frame #18: 0x00000001044a901c postgres`ProcessInterrupts at
postgres.c:3536:3 [opt]
<repeat until we have 174241 frames on the stack, then dump core>
It might be unlikely that a process can be signalled fast enough to
actually fail in this way, but I'm not sure it's impossible, and I
think we should be defending against it. The most trivial recursion
guard would be HOLD_INTERRUPTS()/RESUME_INTERRUPTS() around
ProcessLogMemoryContextInterrupt(), but I think that's probably not
quite good enough because it would make the backend impervious to
pg_terminate_backend() while it's dumping memory contexts, and that
could be a long time if the write blocks.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
@ 2025-05-01 07:53 ` Fujii Masao <[email protected]>
2025-05-02 05:58 ` Re: pgsql: Add function to log the memory contexts of specified backend pro torikoshia <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Fujii Masao @ 2025-05-01 07:53 UTC (permalink / raw)
To: Robert Haas <[email protected]>; Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On 2025/05/01 2:15, Robert Haas wrote:
> On Tue, Apr 6, 2021 at 12:45 AM Fujii Masao <[email protected]> wrote:
>> Add function to log the memory contexts of specified backend process.
>
> Hi,
>
> I think this might need a recursion guard. I tried this:
>
> diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
> index dc4c600922d..b219a934034 100644
> --- a/src/backend/tcop/postgres.c
> +++ b/src/backend/tcop/postgres.c
> @@ -3532,7 +3532,7 @@ ProcessInterrupts(void)
> if (ParallelMessagePending)
> ProcessParallelMessages();
>
> - if (LogMemoryContextPending)
> + if (true)
> ProcessLogMemoryContextInterrupt();
>
> if (PublishMemoryContextPending)
> diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
> index 72f5655fb34..867fd7b0ad5 100644
> --- a/src/include/miscadmin.h
> +++ b/src/include/miscadmin.h
> @@ -112,7 +112,7 @@ extern void ProcessInterrupts(void);
> /* Test whether an interrupt is pending */
> #ifndef WIN32
> #define INTERRUPTS_PENDING_CONDITION() \
> - (unlikely(InterruptPending))
> + (unlikely(InterruptPending) || true)
> #else
> #define INTERRUPTS_PENDING_CONDITION() \
> (unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
> pgwin32_dispatch_queued_signals() : 0, \
>
> That immediately caused infinite recursion, ending in a core dump:
>
> frame #13: 0x0000000104607b00
> postgres`errfinish(filename=<unavailable>, lineno=<unavailable>,
> funcname=<unavailable>) at elog.c:543:2 [opt]
> frame #14: 0x0000000104637078
> postgres`ProcessLogMemoryContextInterrupt at mcxt.c:1392:2 [opt]
> frame #15: 0x00000001044a901c postgres`ProcessInterrupts at
> postgres.c:3536:3 [opt]
> frame #16: 0x0000000104607b54
> postgres`errfinish(filename=<unavailable>, lineno=<unavailable>,
> funcname=<unavailable>) at elog.c:608:2 [opt] [artificial]
> frame #17: 0x0000000104637078
> postgres`ProcessLogMemoryContextInterrupt at mcxt.c:1392:2 [opt]
> frame #18: 0x00000001044a901c postgres`ProcessInterrupts at
> postgres.c:3536:3 [opt]
> <repeat until we have 174241 frames on the stack, then dump core>
>
> It might be unlikely that a process can be signalled fast enough to
> actually fail in this way, but I'm not sure it's impossible, and I
> think we should be defending against it. The most trivial recursion
> guard would be HOLD_INTERRUPTS()/RESUME_INTERRUPTS() around
> ProcessLogMemoryContextInterrupt(), but I think that's probably not
> quite good enough because it would make the backend impervious to
> pg_terminate_backend() while it's dumping memory contexts, and that
> could be a long time if the write blocks.
Just idea, what do you think about adding a flag to indicate whether
ProcessLogMemoryContextInterrupt() is currently running? Then,
when a backend receives a signal and ProcessLogMemoryContextInterrupt()
is invoked, it can simply return immediately if the flag is already set
like this:
------------------------------
@ -1383,8 +1383,14 @@ HandleGetMemoryContextInterrupt(void)
void
ProcessLogMemoryContextInterrupt(void)
{
+ static bool loggingMemoryContext = false;
+
LogMemoryContextPending = false;
+ if (loggingMemoryContext)
+ return;
+ loggingMemoryContext = true;
+
/*
* Use LOG_SERVER_ONLY to prevent this message from being sent to the
* connected client.
@@ -1406,6 +1412,8 @@ ProcessLogMemoryContextInterrupt(void)
* details about individual siblings beyond 100 will not be large.
*/
MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+
+ loggingMemoryContext = false;
}
------------------------------
This way, we can safely ignore overlapping
pg_log_backend_memory_contexts() requests while the function
is already running. Thoughts?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
@ 2025-05-02 05:58 ` torikoshia <[email protected]>
2025-05-03 01:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: torikoshia @ 2025-05-02 05:58 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Robert Haas <[email protected]>; PostgreSQL Hackers <[email protected]>
On 2025-05-02 09:02, Fujii Masao wrote:
> On 2025/05/02 2:27, Fujii Masao wrote:
>>
>>
>> On 2025/05/01 21:42, Robert Haas wrote:
>>> On Thu, May 1, 2025 at 3:53 AM Fujii Masao
>>> <[email protected]> wrote:
>>>> Just idea, what do you think about adding a flag to indicate whether
>>>> ProcessLogMemoryContextInterrupt() is currently running? Then,
>>>> when a backend receives a signal and
>>>> ProcessLogMemoryContextInterrupt()
>>>> is invoked, it can simply return immediately if the flag is already
>>>> set
>>>> like this:
>>>
>>> I think that something like this could work, but you would need more
>>> than this. Otherwise, if the function errors out, the flag would
>>> remain permanently set.
>>
>> Yes, we need to either use PG_TRY()/PG_FINALLY() or handle the flag as
>> a global variable and reset it in the error handling path. I think
>> using
>> PG_TRY()/PG_FINALLY() is the simpler option.
>
> I've implemented the patch in that way. Patch attached.
Thank you for the patch!
I confirmed that with this patch applied, the process no longer crashes
even after applying the change Robert used to trigger the crash.
a small nitpick:
+ * requested repeatedly and rapidly, potentially
leading to infinite
It looks like there are two spaces between "requested" and "repeatedly".
--
Regards,
--
Atsushi Torikoshi
Seconded from NTT DATA GROUP CORPORATION to SRA OSS K.K.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-02 05:58 ` Re: pgsql: Add function to log the memory contexts of specified backend pro torikoshia <[email protected]>
@ 2025-05-03 01:53 ` Fujii Masao <[email protected]>
2025-05-05 14:57 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Fujii Masao @ 2025-05-03 01:53 UTC (permalink / raw)
To: torikoshia <[email protected]>; +Cc: Robert Haas <[email protected]>; PostgreSQL Hackers <[email protected]>
On 2025/05/02 14:58, torikoshia wrote:
> I confirmed that with this patch applied, the process no longer crashes even after applying the change Robert used to trigger the crash.
>
> a small nitpick:
>
> + * requested repeatedly and rapidly, potentially leading to infinite
>
> It looks like there are two spaces between "requested" and "repeatedly".
Thanks for the review and testing! I've fixed the issue you pointed out.
Updated patch attached.
Since git cherry-pick didn't work cleanly for v16 and earlier,
I've also prepared a separate patch for those versions.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From c39b3b83f8a04e781812bd3b83acb235babe3116 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Sat, 3 May 2025 10:21:01 +0900
Subject: [PATCH v2] Add guard to prevent recursive memory contexts logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory contexts logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory contexts logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 60 ++++++++++++++++++++++++-----------
1 file changed, 41 insertions(+), 19 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6f..73dcd64c351 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1198,28 +1198,50 @@ HandleLogMemoryContextInterrupt(void)
void
ProcessLogMemoryContextInterrupt(void)
{
+ static bool in_progress = false;
+
LogMemoryContextPending = false;
- /*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
- */
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ PG_TRY();
+ {
+ /*
+ * Exit immediately if memory contexts logging is already in progress.
+ * This prevents recursive calls, which could occur if logging is
+ * requested repeatedly and rapidly, potentially leading to infinite
+ * recursion and a crash.
+ */
+ if (in_progress)
+ return;
+ in_progress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the number of child contexts to log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the number of child contexts to log per parent to
+ * 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ }
+ PG_FINALLY();
+ {
+ in_progress = false;
+ }
+ PG_END_TRY();
}
void *
--
2.49.0
From 35bb669bcf1be12d7517b3cc993bcf519c0db361 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Sat, 3 May 2025 10:21:01 +0900
Subject: [PATCH v2] Add guard to prevent recursive memory contexts logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory contexts logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory contexts logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 61 +++++++++++++++++++++++------------
1 file changed, 41 insertions(+), 20 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 7d28ca706eb..f0a8ccdfe12 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1383,29 +1383,50 @@ HandleGetMemoryContextInterrupt(void)
void
ProcessLogMemoryContextInterrupt(void)
{
+ static bool in_progress = false;
+
LogMemoryContextPending = false;
- /*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
- */
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ PG_TRY();
+ {
+ /*
+ * Exit immediately if memory contexts logging is already in progress.
+ * This prevents recursive calls, which could occur if logging is
+ * requested repeatedly and rapidly, potentially leading to infinite
+ * recursion and a crash.
+ */
+ if (in_progress)
+ return;
+ in_progress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the depth of the hierarchy, as well as the number of child contexts to
- * log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the depth of the hierarchy, as well as the number of
+ * child contexts to log per parent to 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ }
+ PG_FINALLY();
+ {
+ in_progress = false;
+ }
+ PG_END_TRY();
}
/*
--
2.49.0
Attachments:
[text/plain] v2-0001-PG14-PG16-Add-guard-to-prevent-recursive-memory-contexts-lo.patch (3.8K, ../../[email protected]/2-v2-0001-PG14-PG16-Add-guard-to-prevent-recursive-memory-contexts-lo.patch)
download | inline diff:
From c39b3b83f8a04e781812bd3b83acb235babe3116 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Sat, 3 May 2025 10:21:01 +0900
Subject: [PATCH v2] Add guard to prevent recursive memory contexts logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory contexts logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory contexts logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 60 ++++++++++++++++++++++++-----------
1 file changed, 41 insertions(+), 19 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6f..73dcd64c351 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1198,28 +1198,50 @@ HandleLogMemoryContextInterrupt(void)
void
ProcessLogMemoryContextInterrupt(void)
{
+ static bool in_progress = false;
+
LogMemoryContextPending = false;
- /*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
- */
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ PG_TRY();
+ {
+ /*
+ * Exit immediately if memory contexts logging is already in progress.
+ * This prevents recursive calls, which could occur if logging is
+ * requested repeatedly and rapidly, potentially leading to infinite
+ * recursion and a crash.
+ */
+ if (in_progress)
+ return;
+ in_progress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the number of child contexts to log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the number of child contexts to log per parent to
+ * 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ }
+ PG_FINALLY();
+ {
+ in_progress = false;
+ }
+ PG_END_TRY();
}
void *
--
2.49.0
[text/plain] v2-0001-PG17-master-Add-guard-to-prevent-recursive-memory-contexts-lo.patch (3.9K, ../../[email protected]/3-v2-0001-PG17-master-Add-guard-to-prevent-recursive-memory-contexts-lo.patch)
download | inline diff:
From 35bb669bcf1be12d7517b3cc993bcf519c0db361 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Sat, 3 May 2025 10:21:01 +0900
Subject: [PATCH v2] Add guard to prevent recursive memory contexts logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory contexts logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory contexts logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 61 +++++++++++++++++++++++------------
1 file changed, 41 insertions(+), 20 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 7d28ca706eb..f0a8ccdfe12 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1383,29 +1383,50 @@ HandleGetMemoryContextInterrupt(void)
void
ProcessLogMemoryContextInterrupt(void)
{
+ static bool in_progress = false;
+
LogMemoryContextPending = false;
- /*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
- */
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ PG_TRY();
+ {
+ /*
+ * Exit immediately if memory contexts logging is already in progress.
+ * This prevents recursive calls, which could occur if logging is
+ * requested repeatedly and rapidly, potentially leading to infinite
+ * recursion and a crash.
+ */
+ if (in_progress)
+ return;
+ in_progress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the depth of the hierarchy, as well as the number of child contexts to
- * log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the depth of the hierarchy, as well as the number of
+ * child contexts to log per parent to 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ }
+ PG_FINALLY();
+ {
+ in_progress = false;
+ }
+ PG_END_TRY();
}
/*
--
2.49.0
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-02 05:58 ` Re: pgsql: Add function to log the memory contexts of specified backend pro torikoshia <[email protected]>
2025-05-03 01:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
@ 2025-05-05 14:57 ` Robert Haas <[email protected]>
2025-05-07 09:06 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Robert Haas @ 2025-05-05 14:57 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: torikoshia <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, May 2, 2025 at 9:54 PM Fujii Masao <[email protected]> wrote:
> Thanks for the review and testing! I've fixed the issue you pointed out.
> Updated patch attached.
Thanks for addressing this. However, I believe this commit may need to
take note of the following comment from elog.h:
* Note: if a local variable of the function containing PG_TRY is modified
* in the PG_TRY section and used in the PG_CATCH section, that variable
* must be declared "volatile" for POSIX compliance. This is not mere
* pedantry; we have seen bugs from compilers improperly optimizing code
* away when such a variable was not marked. Beware that gcc's -Wclobbered
* warnings are just about entirely useless for catching such oversights.
Based on this comment, I believe in_progress must be declared volatile.
As a stylistic comment, I think I would prefer making in_progress a
file-level global and giving it a less generic name (e.g.
LogMemoryContextInProgress). However, perhaps others will disagree.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-02 05:58 ` Re: pgsql: Add function to log the memory contexts of specified backend pro torikoshia <[email protected]>
2025-05-03 01:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-05 14:57 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
@ 2025-05-07 09:06 ` Fujii Masao <[email protected]>
2025-07-14 13:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Fujii Masao @ 2025-05-07 09:06 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: torikoshia <[email protected]>; PostgreSQL Hackers <[email protected]>
On 2025/05/05 23:57, Robert Haas wrote:
> On Fri, May 2, 2025 at 9:54 PM Fujii Masao <[email protected]> wrote:
>> Thanks for the review and testing! I've fixed the issue you pointed out.
>> Updated patch attached.
>
> Thanks for addressing this. However, I believe this commit may need to
> take note of the following comment from elog.h:
Thanks for the review!
> * Note: if a local variable of the function containing PG_TRY is modified
> * in the PG_TRY section and used in the PG_CATCH section, that variable
> * must be declared "volatile" for POSIX compliance. This is not mere
> * pedantry; we have seen bugs from compilers improperly optimizing code
> * away when such a variable was not marked. Beware that gcc's -Wclobbered
> * warnings are just about entirely useless for catching such oversights.
>
> Based on this comment, I believe in_progress must be declared volatile.
You're right. OTOH, setting the flag inside the PG_TRY() block isn't necessary,
so I've moved it outside instead of leaving it inside and marking the flag volatile.
> As a stylistic comment, I think I would prefer making in_progress a
> file-level global and giving it a less generic name (e.g.
> LogMemoryContextInProgress). However, perhaps others will disagree.
I'm fine with this. I've renamed the flag and made it a file-level global
variable as suggested. Updated patch is attached.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From c5870f8b41118d1208d111101b9a360f1abc7e53 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 7 May 2025 17:45:02 +0900
Subject: [PATCH v3] Add guard to prevent recursive memory context logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory context logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory context logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 57 ++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6f..30286a3ff33 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -149,6 +149,9 @@ MemoryContext CurTransactionContext = NULL;
/* This is a transient link to the active portal's memory context: */
MemoryContext PortalContext = NULL;
+/* Is memory context logging currently in progress? */
+static bool LogMemoryContextInProgress = false;
+
static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
bool print, int max_children,
@@ -1201,25 +1204,45 @@ ProcessLogMemoryContextInterrupt(void)
LogMemoryContextPending = false;
/*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
+ * Exit immediately if memory context logging is already in progress. This
+ * prevents recursive calls, which could occur if logging is requested
+ * repeatedly and rapidly, potentially leading to infinite recursion and a
+ * crash.
*/
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ if (LogMemoryContextInProgress)
+ return;
+ LogMemoryContextInProgress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the number of child contexts to log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ PG_TRY();
+ {
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the number of child contexts to log per parent to
+ * 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ }
+ PG_FINALLY();
+ {
+ LogMemoryContextInProgress = false;
+ }
+ PG_END_TRY();
}
void *
--
2.49.0
From 92a0fc0397831849f6e1443b010733a69e1d4b1d Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 7 May 2025 16:49:31 +0900
Subject: [PATCH v3] Add guard to prevent recursive memory context logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory context logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory context logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 58 ++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 18 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 7d28ca706eb..4690d039011 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -174,6 +174,9 @@ MemoryContext CurTransactionContext = NULL;
MemoryContext PortalContext = NULL;
dsa_area *MemoryStatsDsaArea = NULL;
+/* Is memory context logging currently in progress? */
+static bool LogMemoryContextInProgress = false;
+
static void MemoryContextDeleteOnly(MemoryContext context);
static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
@@ -1386,26 +1389,45 @@ ProcessLogMemoryContextInterrupt(void)
LogMemoryContextPending = false;
/*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
+ * Exit immediately if memory context logging is already in progress. This
+ * prevents recursive calls, which could occur if logging is requested
+ * repeatedly and rapidly, potentially leading to infinite recursion and a
+ * crash.
*/
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ if (LogMemoryContextInProgress)
+ return;
+ LogMemoryContextInProgress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the depth of the hierarchy, as well as the number of child contexts to
- * log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ PG_TRY();
+ {
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the depth of the hierarchy, as well as the number of
+ * child contexts to log per parent to 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ }
+ PG_FINALLY();
+ {
+ LogMemoryContextInProgress = false;
+ }
+ PG_END_TRY();
}
/*
--
2.49.0
Attachments:
[text/plain] v3-0001-PG14-PG16-Add-guard-to-prevent-recursive-memory-context-log.patch (4.2K, ../../[email protected]/2-v3-0001-PG14-PG16-Add-guard-to-prevent-recursive-memory-context-log.patch)
download | inline diff:
From c5870f8b41118d1208d111101b9a360f1abc7e53 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 7 May 2025 17:45:02 +0900
Subject: [PATCH v3] Add guard to prevent recursive memory context logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory context logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory context logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 57 ++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6f..30286a3ff33 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -149,6 +149,9 @@ MemoryContext CurTransactionContext = NULL;
/* This is a transient link to the active portal's memory context: */
MemoryContext PortalContext = NULL;
+/* Is memory context logging currently in progress? */
+static bool LogMemoryContextInProgress = false;
+
static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
bool print, int max_children,
@@ -1201,25 +1204,45 @@ ProcessLogMemoryContextInterrupt(void)
LogMemoryContextPending = false;
/*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
+ * Exit immediately if memory context logging is already in progress. This
+ * prevents recursive calls, which could occur if logging is requested
+ * repeatedly and rapidly, potentially leading to infinite recursion and a
+ * crash.
*/
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ if (LogMemoryContextInProgress)
+ return;
+ LogMemoryContextInProgress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the number of child contexts to log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ PG_TRY();
+ {
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the number of child contexts to log per parent to
+ * 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ }
+ PG_FINALLY();
+ {
+ LogMemoryContextInProgress = false;
+ }
+ PG_END_TRY();
}
void *
--
2.49.0
[text/plain] v3-0001-PG17-master-Add-guard-to-prevent-recursive-memory-context-log.patch (4.3K, ../../[email protected]/3-v3-0001-PG17-master-Add-guard-to-prevent-recursive-memory-context-log.patch)
download | inline diff:
From 92a0fc0397831849f6e1443b010733a69e1d4b1d Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 7 May 2025 16:49:31 +0900
Subject: [PATCH v3] Add guard to prevent recursive memory context logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory context logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory context logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 58 ++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 18 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 7d28ca706eb..4690d039011 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -174,6 +174,9 @@ MemoryContext CurTransactionContext = NULL;
MemoryContext PortalContext = NULL;
dsa_area *MemoryStatsDsaArea = NULL;
+/* Is memory context logging currently in progress? */
+static bool LogMemoryContextInProgress = false;
+
static void MemoryContextDeleteOnly(MemoryContext context);
static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
@@ -1386,26 +1389,45 @@ ProcessLogMemoryContextInterrupt(void)
LogMemoryContextPending = false;
/*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
+ * Exit immediately if memory context logging is already in progress. This
+ * prevents recursive calls, which could occur if logging is requested
+ * repeatedly and rapidly, potentially leading to infinite recursion and a
+ * crash.
*/
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ if (LogMemoryContextInProgress)
+ return;
+ LogMemoryContextInProgress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the depth of the hierarchy, as well as the number of child contexts to
- * log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ PG_TRY();
+ {
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the depth of the hierarchy, as well as the number of
+ * child contexts to log per parent to 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ }
+ PG_FINALLY();
+ {
+ LogMemoryContextInProgress = false;
+ }
+ PG_END_TRY();
}
/*
--
2.49.0
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-02 05:58 ` Re: pgsql: Add function to log the memory contexts of specified backend pro torikoshia <[email protected]>
2025-05-03 01:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-05 14:57 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-07 09:06 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
@ 2025-07-14 13:53 ` Fujii Masao <[email protected]>
2025-12-08 19:32 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Artem Gavrilov <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Fujii Masao @ 2025-07-14 13:53 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: torikoshia <[email protected]>; PostgreSQL Hackers <[email protected]>
On 2025/05/07 18:06, Fujii Masao wrote:
>
>
> On 2025/05/05 23:57, Robert Haas wrote:
>> On Fri, May 2, 2025 at 9:54 PM Fujii Masao <[email protected]> wrote:
>>> Thanks for the review and testing! I've fixed the issue you pointed out.
>>> Updated patch attached.
>>
>> Thanks for addressing this. However, I believe this commit may need to
>> take note of the following comment from elog.h:
>
> Thanks for the review!
>
>
>> * Note: if a local variable of the function containing PG_TRY is modified
>> * in the PG_TRY section and used in the PG_CATCH section, that variable
>> * must be declared "volatile" for POSIX compliance. This is not mere
>> * pedantry; we have seen bugs from compilers improperly optimizing code
>> * away when such a variable was not marked. Beware that gcc's -Wclobbered
>> * warnings are just about entirely useless for catching such oversights.
>>
>> Based on this comment, I believe in_progress must be declared volatile.
>
> You're right. OTOH, setting the flag inside the PG_TRY() block isn't necessary,
> so I've moved it outside instead of leaving it inside and marking the flag volatile.
>
>
>> As a stylistic comment, I think I would prefer making in_progress a
>> file-level global and giving it a less generic name (e.g.
>> LogMemoryContextInProgress). However, perhaps others will disagree.
>
> I'm fine with this. I've renamed the flag and made it a file-level global
> variable as suggested. Updated patch is attached.
I've attached the rebased versions of the patches.
The patch for v14–v16 is labeled with a .txt extension to prevent cfbot
from treating it as a patch for master, which would cause it to fail
when applying.
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
From 3c6e6d0bab29147fb1dc469161b619b977cf5fc8 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Mon, 14 Jul 2025 22:42:44 +0900
Subject: [PATCH v4] Add guard to prevent recursive memory context logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory context logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory context logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 57 ++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index a5f31e23a02..ffdab7b087d 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -56,6 +56,9 @@ MemoryContext CurTransactionContext = NULL;
/* This is a transient link to the active portal's memory context: */
MemoryContext PortalContext = NULL;
+/* Is memory context logging currently in progress? */
+static bool LogMemoryContextInProgress = false;
+
static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
bool print, int max_children,
@@ -1043,25 +1046,45 @@ ProcessLogMemoryContextInterrupt(void)
LogMemoryContextPending = false;
/*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
+ * Exit immediately if memory context logging is already in progress. This
+ * prevents recursive calls, which could occur if logging is requested
+ * repeatedly and rapidly, potentially leading to infinite recursion and a
+ * crash.
*/
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ if (LogMemoryContextInProgress)
+ return;
+ LogMemoryContextInProgress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the number of child contexts to log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ PG_TRY();
+ {
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the number of child contexts to log per parent to
+ * 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ }
+ PG_FINALLY();
+ {
+ LogMemoryContextInProgress = false;
+ }
+ PG_END_TRY();
}
void *
--
2.49.0
From 2b9e286c5d98bf24a3efc06aeb7645990c2ea7c0 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Mon, 14 Jul 2025 22:38:45 +0900
Subject: [PATCH v4] Add guard to prevent recursive memory context logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory context logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory context logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 58 ++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 18 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 15fa4d0a55e..f7689ea6de8 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -157,6 +157,9 @@ MemoryContext CurTransactionContext = NULL;
/* This is a transient link to the active portal's memory context: */
MemoryContext PortalContext = NULL;
+/* Is memory context logging currently in progress? */
+static bool LogMemoryContextInProgress = false;
+
static void MemoryContextDeleteOnly(MemoryContext context);
static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
@@ -1295,26 +1298,45 @@ ProcessLogMemoryContextInterrupt(void)
LogMemoryContextPending = false;
/*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
+ * Exit immediately if memory context logging is already in progress. This
+ * prevents recursive calls, which could occur if logging is requested
+ * repeatedly and rapidly, potentially leading to infinite recursion and a
+ * crash.
*/
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ if (LogMemoryContextInProgress)
+ return;
+ LogMemoryContextInProgress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the depth of the hierarchy, as well as the number of child contexts to
- * log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ PG_TRY();
+ {
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the depth of the hierarchy, as well as the number of
+ * child contexts to log per parent to 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ }
+ PG_FINALLY();
+ {
+ LogMemoryContextInProgress = false;
+ }
+ PG_END_TRY();
}
void *
--
2.49.0
Attachments:
[text/plain] v4-0001-PG14-PG16-Add-guard-to-prevent-recursive-memory-context-log.txt (4.2K, ../../[email protected]/2-v4-0001-PG14-PG16-Add-guard-to-prevent-recursive-memory-context-log.txt)
download | inline diff:
From 3c6e6d0bab29147fb1dc469161b619b977cf5fc8 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Mon, 14 Jul 2025 22:42:44 +0900
Subject: [PATCH v4] Add guard to prevent recursive memory context logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory context logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory context logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 57 ++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index a5f31e23a02..ffdab7b087d 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -56,6 +56,9 @@ MemoryContext CurTransactionContext = NULL;
/* This is a transient link to the active portal's memory context: */
MemoryContext PortalContext = NULL;
+/* Is memory context logging currently in progress? */
+static bool LogMemoryContextInProgress = false;
+
static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
bool print, int max_children,
@@ -1043,25 +1046,45 @@ ProcessLogMemoryContextInterrupt(void)
LogMemoryContextPending = false;
/*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
+ * Exit immediately if memory context logging is already in progress. This
+ * prevents recursive calls, which could occur if logging is requested
+ * repeatedly and rapidly, potentially leading to infinite recursion and a
+ * crash.
*/
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ if (LogMemoryContextInProgress)
+ return;
+ LogMemoryContextInProgress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the number of child contexts to log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ PG_TRY();
+ {
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the number of child contexts to log per parent to
+ * 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, false);
+ }
+ PG_FINALLY();
+ {
+ LogMemoryContextInProgress = false;
+ }
+ PG_END_TRY();
}
void *
--
2.49.0
[text/plain] v4-0001-PG17-master-Add-guard-to-prevent-recursive-memory-context-log.patch (4.3K, ../../[email protected]/3-v4-0001-PG17-master-Add-guard-to-prevent-recursive-memory-context-log.patch)
download | inline diff:
From 2b9e286c5d98bf24a3efc06aeb7645990c2ea7c0 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Mon, 14 Jul 2025 22:38:45 +0900
Subject: [PATCH v4] Add guard to prevent recursive memory context logging.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if memory context logging was triggered repeatedly and
rapidly while a previous request was still being processed, it could
result in recursive calls to ProcessLogMemoryContextInterrupt().
This could lead to infinite recursion and potentially crash the process.
This commit adds a guard to prevent such recursion.
If ProcessLogMemoryContextInterrupt() is already in progress and
logging memory contexts, subsequent calls will exit immediately,
avoiding unintended recursive calls.
While this scenario is unlikely in practice, it’s not impossible.
This change adds a safety check to prevent such failures.
Back-patch to v14, where memory context logging was introduced.
Reported-by: Robert Haas <[email protected]>
Author: Fujii Masao <[email protected]>
Reviewed-by: Atsushi Torikoshi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/CA+TgmoZMrv32tbNRrFTvF9iWLnTGqbhYSLVcrHGuwZvCtph0NA@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/mmgr/mcxt.c | 58 ++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 18 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 15fa4d0a55e..f7689ea6de8 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -157,6 +157,9 @@ MemoryContext CurTransactionContext = NULL;
/* This is a transient link to the active portal's memory context: */
MemoryContext PortalContext = NULL;
+/* Is memory context logging currently in progress? */
+static bool LogMemoryContextInProgress = false;
+
static void MemoryContextDeleteOnly(MemoryContext context);
static void MemoryContextCallResetCallbacks(MemoryContext context);
static void MemoryContextStatsInternal(MemoryContext context, int level,
@@ -1295,26 +1298,45 @@ ProcessLogMemoryContextInterrupt(void)
LogMemoryContextPending = false;
/*
- * Use LOG_SERVER_ONLY to prevent this message from being sent to the
- * connected client.
+ * Exit immediately if memory context logging is already in progress. This
+ * prevents recursive calls, which could occur if logging is requested
+ * repeatedly and rapidly, potentially leading to infinite recursion and a
+ * crash.
*/
- ereport(LOG_SERVER_ONLY,
- (errhidestmt(true),
- errhidecontext(true),
- errmsg("logging memory contexts of PID %d", MyProcPid)));
+ if (LogMemoryContextInProgress)
+ return;
+ LogMemoryContextInProgress = true;
- /*
- * When a backend process is consuming huge memory, logging all its memory
- * contexts might overrun available disk space. To prevent this, we limit
- * the depth of the hierarchy, as well as the number of child contexts to
- * log per parent to 100.
- *
- * As with MemoryContextStats(), we suppose that practical cases where the
- * dump gets long will typically be huge numbers of siblings under the
- * same parent context; while the additional debugging value from seeing
- * details about individual siblings beyond 100 will not be large.
- */
- MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ PG_TRY();
+ {
+ /*
+ * Use LOG_SERVER_ONLY to prevent this message from being sent to the
+ * connected client.
+ */
+ ereport(LOG_SERVER_ONLY,
+ (errhidestmt(true),
+ errhidecontext(true),
+ errmsg("logging memory contexts of PID %d", MyProcPid)));
+
+ /*
+ * When a backend process is consuming huge memory, logging all its
+ * memory contexts might overrun available disk space. To prevent
+ * this, we limit the depth of the hierarchy, as well as the number of
+ * child contexts to log per parent to 100.
+ *
+ * As with MemoryContextStats(), we suppose that practical cases where
+ * the dump gets long will typically be huge numbers of siblings under
+ * the same parent context; while the additional debugging value from
+ * seeing details about individual siblings beyond 100 will not be
+ * large.
+ */
+ MemoryContextStatsDetail(TopMemoryContext, 100, 100, false);
+ }
+ PG_FINALLY();
+ {
+ LogMemoryContextInProgress = false;
+ }
+ PG_END_TRY();
}
void *
--
2.49.0
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-02 05:58 ` Re: pgsql: Add function to log the memory contexts of specified backend pro torikoshia <[email protected]>
2025-05-03 01:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-05 14:57 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-07 09:06 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-07-14 13:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
@ 2025-12-08 19:32 ` Artem Gavrilov <[email protected]>
2025-12-15 15:07 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Artem Gavrilov @ 2025-12-08 19:32 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Robert Haas <[email protected]>; torikoshia <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi Fujii,
I did your patch review. It successfully applies to all targeted branches:
REL_14_STABLE (48969555447), REL_15_STABLE (b9a02b9780b), REL_16_STABLE
(4d689a17693), REL_17_STABLE (cad40cec24f), REL_18_STABLE (5278222853c) and
master (d0d0ba6cf66). All tests successfully pass on MacOS 15.7 for every
revision. Everything seems fine with the patch, I think it's ready for
committer.
--
Artem Gavrilov
Senior Software Engineer, Percona
[email protected]
percona.com <http://www.percona.com;
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-02 05:58 ` Re: pgsql: Add function to log the memory contexts of specified backend pro torikoshia <[email protected]>
2025-05-03 01:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-05 14:57 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-07 09:06 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-07-14 13:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-12-08 19:32 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Artem Gavrilov <[email protected]>
@ 2025-12-15 15:07 ` Robert Haas <[email protected]>
2025-12-19 03:17 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Robert Haas @ 2025-12-15 15:07 UTC (permalink / raw)
To: Artem Gavrilov <[email protected]>; +Cc: Fujii Masao <[email protected]>; torikoshia <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Dec 8, 2025 at 2:32 PM Artem Gavrilov
<[email protected]> wrote:
> I did your patch review. It successfully applies to all targeted branches: REL_14_STABLE (48969555447), REL_15_STABLE (b9a02b9780b), REL_16_STABLE (4d689a17693), REL_17_STABLE (cad40cec24f), REL_18_STABLE (5278222853c) and master (d0d0ba6cf66). All tests successfully pass on MacOS 15.7 for every revision. Everything seems fine with the patch, I think it's ready for committer.
LGTM, too.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Add function to log the memory contexts of specified backend pro
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-02 05:58 ` Re: pgsql: Add function to log the memory contexts of specified backend pro torikoshia <[email protected]>
2025-05-03 01:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-05 14:57 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-07 09:06 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-07-14 13:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-12-08 19:32 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Artem Gavrilov <[email protected]>
2025-12-15 15:07 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
@ 2025-12-19 03:17 ` Fujii Masao <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Fujii Masao @ 2025-12-19 03:17 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Artem Gavrilov <[email protected]>; Fujii Masao <[email protected]>; torikoshia <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Dec 16, 2025 at 12:07 AM Robert Haas <[email protected]> wrote:
>
> On Mon, Dec 8, 2025 at 2:32 PM Artem Gavrilov
> <[email protected]> wrote:
> > I did your patch review. It successfully applies to all targeted branches: REL_14_STABLE (48969555447), REL_15_STABLE (b9a02b9780b), REL_16_STABLE (4d689a17693), REL_17_STABLE (cad40cec24f), REL_18_STABLE (5278222853c) and master (d0d0ba6cf66). All tests successfully pass on MacOS 15.7 for every revision. Everything seems fine with the patch, I think it's ready for committer.
>
> LGTM, too.
Thanks to both of you for the review! I've pushed the patch.
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2025-12-19 03:17 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18 16:19 [PATCH v45 1/3] Subscripting for jsonb Dmitrii Dolgov <[email protected]>
2021-04-06 04:45 pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-04-30 17:15 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-01 07:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-02 05:58 ` Re: pgsql: Add function to log the memory contexts of specified backend pro torikoshia <[email protected]>
2025-05-03 01:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-05-05 14:57 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-05-07 09:06 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-07-14 13:53 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
2025-12-08 19:32 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Artem Gavrilov <[email protected]>
2025-12-15 15:07 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Robert Haas <[email protected]>
2025-12-19 03:17 ` Re: pgsql: Add function to log the memory contexts of specified backend pro Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox