public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v38] Subscripting for jsonb
16+ messages / 7 participants
[nested] [flat]

* [PATCH v38] Subscripting for jsonb
@ 2020-12-18 16:19 Dmitrii Dolgov <[email protected]>
  0 siblings, 0 replies; 16+ 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 of type object 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   | 282 ++++++++++++++++++++++++++++
 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 | 273 ++++++++++++++++++++++++++-
 src/test/regress/sql/jsonb.sql      |  84 ++++++++-
 10 files changed, 852 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..cad7b02559 100644
--- a/doc/src/sgml/json.sgml
+++ b/doc/src/sgml/json.sgml
@@ -602,6 +602,54 @@ SELECT jdoc-&gt;'guid', jdoc-&gt;'name' FROM api WHERE jdoc @&gt; '{"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": 1}
+UPDATE table_name SET jsonb_field[1] = '1';
+</programlisting>
+
+  </para>
+ </sect2>
+
  <sect2>
   <title>Transforms</title>
 
diff --git a/src/backend/utils/adt/Makefile b/src/backend/utils/adt/Makefile
index ce09ad7375..0ef0f36e36 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..b58efb7c0d
--- /dev/null
+++ b/src/backend/utils/adt/jsonbsubs.c
@@ -0,0 +1,282 @@
+/*-------------------------------------------------------------------------
+ *
+ * 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"
+
+
+/*
+ * 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)
+		{
+			subexpr = transformExpr(pstate, ai->uidx, pstate->p_expr_kind);
+			subexpr = coerce_to_target_type(pstate,
+											subexpr, exprType(subexpr),
+											TEXTOID, -1,
+											COERCION_ASSIGNMENT,
+											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 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;
+
+	/* 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;
+			}
+		}
+	}
+
+	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;
+	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,
+									  sbsrefstate->upperindex,
+									  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;
+	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 of object
+	 * type and proceed with the assignment.
+	 */
+	if (*op->resnull)
+	{
+		JsonbValue *newSource = (JsonbValue *) palloc(sizeof(JsonbValue));
+		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,
+									  sbsrefstate->upperindex,
+									  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)
+{
+	/*
+	 * 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 12557ce3af..1925cd8026 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.
  */
@@ -4153,58 +4201,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)
  *
@@ -4476,7 +4472,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;
@@ -4634,7 +4631,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;
@@ -4792,10 +4790,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;
@@ -4848,11 +4846,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])
@@ -4869,7 +4867,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++)
@@ -4900,7 +4898,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;
 			}
@@ -4923,7 +4921,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);
@@ -4955,7 +4953,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,
@@ -5003,7 +5001,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;
 	}
 
@@ -5019,7 +5017,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
@@ -5030,7 +5028,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;
 			}
@@ -5064,7 +5062,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 e6c7b070f6..82570cce6f 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10943,6 +10943,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 28240bdce3..3c0ee2a81b 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 a70cd0b7c1..85f8364d70 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -4567,7 +4567,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          
@@ -4697,6 +4697,277 @@ 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];
+ jsonb 
+-------
+ 
+(1 row)
+
+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 | {"0": 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 3e2b8f66df..5b8a51f712 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -1172,7 +1172,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"');
@@ -1203,6 +1203,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


--hletpuqfhzttgv77--





^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
@ 2023-06-11 01:44 Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Bharath Rupireddy @ 2023-06-11 01:44 UTC (permalink / raw)
  To: Kyotaro Horiguchi <[email protected]>; Michael Paquier <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; PostgreSQL Hackers <[email protected]>

On Wed, Aug 24, 2022 at 6:42 AM Kyotaro Horiguchi
<[email protected]> wrote:
>
> At Tue, 16 Aug 2022 18:40:49 +0200, Alvaro Herrera <[email protected]> wrote in
> > On 2022-Aug-16, Andrew Dunstan wrote:
> >
> > > I don't think there's a hard and fast rule about it. Certainly the case
> > > would be more compelling if the functions were used across different TAP
> > > suites. The SSL suite has suite-specific modules. That's a pattern also
> > > worth considering. e.g something like.
> > >
> > >     use FindBin qw($Bin);
> > >     use lib $Bin;
> > >     use MySuite;
> > >
> > > and then you put your common routines in MySuite.pm in the same
> > > directory as the TAP test files.
> >
> > Yeah, I agree with that for advance_wal.  Regarding find_in_log, that
> > one seems general enough to warrant being in Cluster.pm -- consider
> > issues_sql_like, which also slurps_file($log).  That could be unified a
> > little bit, I think.
>
> +1

With the generalized function for find_in_log() has been added as part
of https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=e25e5f7fc6b74c9d4ce82627e9145ef55...,
I'm proposing a generalized function for advance_wal(). Please find
the attached patch.

I tried to replace the existing tests with the new cluster function
advance_wal(). Please let me know if I'm missing any other tests.
Also, this new function can be used by an in-progress feature -
https://commitfest.postgresql.org/43/3663/.

Thoughts?

FWIW, it's discussed here -
https://www.postgresql.org/message-id/ZIKVd%2Ba43UfsIWJE%40paquier.xyz.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com


Attachments:

  [application/x-patch] v1-0001-Add-a-perl-function-in-Cluster.pm-to-generate-WAL.patch (7.1K, ../../CALj2ACU3R8QFCvDewHCMKjgb2w_-CMCyd6DAK=Jb-af14da5eg@mail.gmail.com/2-v1-0001-Add-a-perl-function-in-Cluster.pm-to-generate-WAL.patch)
  download | inline diff:
From ab7a5b56e0ed84fb0cceef1892dc435fed877029 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Sat, 10 Jun 2023 08:05:20 +0000
Subject: [PATCH v1] Add a perl function in Cluster.pm to generate WAL

This commit adds a perl function in Cluster.pm to generate WAL.
Some TAP tests are now using their own way to generate WAL.
Generalizing this functionality enables multiple TAP tests to
reuse the functionality.
---
 src/test/perl/PostgreSQL/Test/Cluster.pm      | 20 ++++++++
 src/test/recovery/t/001_stream_rep.pl         |  6 +--
 src/test/recovery/t/019_replslot_limit.pl     | 46 ++++---------------
 .../t/035_standby_logical_decoding.pl         |  7 +--
 4 files changed, 33 insertions(+), 46 deletions(-)

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 5e161dbee6..cf2202d170 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -3120,6 +3120,26 @@ sub create_logical_slot_on_standby
 
 =pod
 
+=item $node->advance_wal($n)
+
+Advance WAL of given node by $n segments
+
+=cut
+
+sub advance_wal
+{
+	my ($self, $n) = @_;
+
+	# Advance by $n segments (= (wal_segment_size * $n) bytes).
+	for (my $i = 0; $i < $n; $i++)
+	{
+		$self->safe_psql('postgres',
+			"CREATE TABLE tt (); DROP TABLE tt; SELECT pg_switch_wal();");
+	}
+}
+
+=pod
+
 =back
 
 =cut
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index 0c72ba0944..0e256dab8d 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -496,11 +496,7 @@ $node_primary->safe_psql('postgres',
 my $segment_removed = $node_primary->safe_psql('postgres',
 	'SELECT pg_walfile_name(pg_current_wal_lsn())');
 chomp($segment_removed);
-$node_primary->psql(
-	'postgres', "
-	CREATE TABLE tab_phys_slot (a int);
-	INSERT INTO tab_phys_slot VALUES (generate_series(1,10));
-	SELECT pg_switch_wal();");
+$node_primary->advance_wal(1);
 my $current_lsn =
   $node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
 chomp($current_lsn);
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 33e50ad933..a6e427ebf1 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -59,7 +59,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved|t", 'check the catching-up state');
 
 # Advance WAL by five segments (= 5MB) on primary
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is always "safe" when fitting max_wal_size
@@ -69,7 +69,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved|t",
 	'check that it is safe if WAL fits in max_wal_size');
 
-advance_wal($node_primary, 4);
+$node_primary->advance_wal(4);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is always "safe" when max_slot_wal_keep_size is not set
@@ -100,7 +100,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved", 'check that max_slot_wal_keep_size is working');
 
 # Advance WAL again then checkpoint, reducing remain by 2 MB.
-advance_wal($node_primary, 2);
+$node_primary->advance_wal(2);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is still working
@@ -118,7 +118,7 @@ $node_standby->stop;
 $result = $node_primary->safe_psql('postgres',
 	"ALTER SYSTEM SET wal_keep_size to '8MB'; SELECT pg_reload_conf();");
 # Advance WAL again then checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
 $result = $node_primary->safe_psql('postgres',
 	"SELECT wal_status as remain FROM pg_replication_slots WHERE slot_name = 'rep1'"
 );
@@ -134,7 +134,7 @@ $node_primary->wait_for_catchup($node_standby);
 $node_standby->stop;
 
 # Advance WAL again without checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
 
 # Slot gets into 'reserved' state
 $result = $node_primary->safe_psql('postgres',
@@ -145,7 +145,7 @@ is($result, "extended", 'check that the slot state changes to "extended"');
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # Advance WAL again without checkpoint; remain goes to 0.
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
 
 # Slot gets into 'unreserved' state and safe_wal_size is negative
 $result = $node_primary->safe_psql('postgres',
@@ -174,7 +174,7 @@ $node_primary->safe_psql('postgres',
 
 # Advance WAL again. The slot loses the oldest segment by the next checkpoint
 my $logstart = get_log_size($node_primary);
-advance_wal($node_primary, 7);
+$node_primary->advance_wal(7);
 
 # Now create another checkpoint and wait until the WARNING is issued
 $node_primary->safe_psql('postgres',
@@ -275,19 +275,8 @@ $node_standby->init_from_backup($node_primary2, $backup_name,
 	has_streaming => 1);
 $node_standby->append_conf('postgresql.conf', "primary_slot_name = 'rep1'");
 $node_standby->start;
-my @result =
-  split(
-	'\n',
-	$node_primary2->safe_psql(
-		'postgres',
-		"CREATE TABLE tt();
-		 DROP TABLE tt;
-		 SELECT pg_switch_wal();
-		 CHECKPOINT;
-		 SELECT 'finished';",
-		timeout => $PostgreSQL::Test::Utils::timeout_default));
-is($result[1], 'finished', 'check if checkpoint command is not blocked');
-
+$node_primary2->advance_wal(1);
+$node_primary2->safe_psql('postgres', 'CHECKPOINT;');
 $node_primary2->stop;
 $node_standby->stop;
 
@@ -372,7 +361,7 @@ $logstart = get_log_size($node_primary3);
 # freeze walsender and walreceiver. Slot will still be active, but walreceiver
 # won't get anything anymore.
 kill 'STOP', $senderpid, $receiverpid;
-advance_wal($node_primary3, 2);
+$node_primary3->advance_wal(2);
 
 my $msg_logged = 0;
 my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
@@ -418,21 +407,6 @@ kill 'CONT', $receiverpid;
 $node_primary3->stop;
 $node_standby3->stop;
 
-#####################################
-# Advance WAL of $node by $n segments
-sub advance_wal
-{
-	my ($node, $n) = @_;
-
-	# Advance by $n segments (= (wal_segment_size * $n) bytes) on primary.
-	for (my $i = 0; $i < $n; $i++)
-	{
-		$node->safe_psql('postgres',
-			"CREATE TABLE t (); DROP TABLE t; SELECT pg_switch_wal();");
-	}
-	return;
-}
-
 # return the size of logfile of $node in bytes
 sub get_log_size
 {
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 480e6d6caa..2d19adab4e 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -524,11 +524,8 @@ my $walfile_name = $node_primary->safe_psql('postgres',
 chomp($walfile_name);
 
 # Generate some activity and switch WAL file on the primary
-$node_primary->safe_psql(
-	'postgres', "create table retain_test(a int);
-									 select pg_switch_wal();
-									 insert into retain_test values(1);
-									 checkpoint;");
+$node_primary->advance_wal(1);
+$node_primary->safe_psql('postgres', "checkpoint;");
 
 # Wait for the standby to catch up
 $node_primary->wait_for_replay_catchup($node_standby);
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
@ 2023-06-15 04:40 ` Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Kyotaro Horiguchi @ 2023-06-15 04:40 UTC (permalink / raw)
  To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]

Mmm. It seems like the email I thought I'd sent failed to go out.

At Sun, 11 Jun 2023 07:14:54 +0530, Bharath Rupireddy <[email protected]> wrote in 
> On Wed, Aug 24, 2022 at 6:42 AM Kyotaro Horiguchi
> <[email protected]> wrote:
> >
> > At Tue, 16 Aug 2022 18:40:49 +0200, Alvaro Herrera <[email protected]> wrote in
> > > On 2022-Aug-16, Andrew Dunstan wrote:
> > > Yeah, I agree with that for advance_wal.  Regarding find_in_log, that
> > > one seems general enough to warrant being in Cluster.pm -- consider
> > > issues_sql_like, which also slurps_file($log).  That could be unified a
> > > little bit, I think.
> >
> > +1
> 
> With the generalized function for find_in_log() has been added as part
> of https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=e25e5f7fc6b74c9d4ce82627e9145ef55...,
> I'm proposing a generalized function for advance_wal(). Please find
> the attached patch.
> 
> I tried to replace the existing tests with the new cluster function
> advance_wal(). Please let me know if I'm missing any other tests.
> Also, this new function can be used by an in-progress feature -
> https://commitfest.postgresql.org/43/3663/.
> 
> Thoughts?

Thanks!

+			"CREATE TABLE tt (); DROP TABLE tt; SELECT pg_switch_wal();");

At least since 11, we can utilize pg_logical_emit_message() for this
purpose. It's more lightweight and seems appropriate, not only because
it doesn't cause any side effects but also bacause we don't have to
worry about name conflicts.


-		 SELECT 'finished';",
-		timeout => $PostgreSQL::Test::Utils::timeout_default));
-is($result[1], 'finished', 'check if checkpoint command is not blocked');
-
+$node_primary2->advance_wal(1);
+$node_primary2->safe_psql('postgres', 'CHECKPOINT;');

This test anticipates that the checkpoint could get blocked. Shouldn't
we keep the timeout?


-$node_primary->safe_psql(
-	'postgres', "create table retain_test(a int);
-									 select pg_switch_wal();
-									 insert into retain_test values(1);
-									 checkpoint;");
+$node_primary->advance_wal(1);
+$node_primary->safe_psql('postgres', "checkpoint;");

The original test generated some WAL after the segment switch, which
appears to be a significant characteristics of the test.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
@ 2023-06-16 02:30   ` Michael Paquier <[email protected]>
  2023-06-16 04:30     ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  0 siblings, 2 replies; 16+ messages in thread

From: Michael Paquier @ 2023-06-16 02:30 UTC (permalink / raw)
  To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]

On Thu, Jun 15, 2023 at 01:40:15PM +0900, Kyotaro Horiguchi wrote:
> +			"CREATE TABLE tt (); DROP TABLE tt; SELECT pg_switch_wal();");
> 
> At least since 11, we can utilize pg_logical_emit_message() for this
> purpose. It's more lightweight and seems appropriate, not only because
> it doesn't cause any side effects but also bacause we don't have to
> worry about name conflicts.

Making this as cheap as possible by design is a good concept for a
common routine.  +1.

> -		 SELECT 'finished';",
> -		timeout => $PostgreSQL::Test::Utils::timeout_default));
> -is($result[1], 'finished', 'check if checkpoint command is not blocked');
> -
> +$node_primary2->advance_wal(1);
> +$node_primary2->safe_psql('postgres', 'CHECKPOINT;');
> 
> This test anticipates that the checkpoint could get blocked. Shouldn't
> we keep the timeout?

Indeed, this would partially invalidate what's getting tested in light
of 1816a1c6 where we run a secondary command after the checkpoint.  So
the last SELECT should remain around.

> -$node_primary->safe_psql(
> -    'postgres', "create table retain_test(a int);
> -                                     select pg_switch_wal();
> -                                     insert into retain_test values(1);
> -                                     checkpoint;");
> +$node_primary->advance_wal(1);
> +$node_primary->safe_psql('postgres', "checkpoint;");
> 
> The original test generated some WAL after the segment switch, which
> appears to be a significant characteristics of the test.

Still it does not matter for this specific case?  The logical slot has
been already invalidated, so we don't care much about logical changes
in WAL, do we?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../ZIvJN%[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
@ 2023-06-16 04:30     ` Kyotaro Horiguchi <[email protected]>
  1 sibling, 0 replies; 16+ messages in thread

From: Kyotaro Horiguchi @ 2023-06-16 04:30 UTC (permalink / raw)
  To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]

Thanks for the comments.

At Fri, 16 Jun 2023 11:30:15 +0900, Michael Paquier <[email protected]> wrote in 
> > -$node_primary->safe_psql(
> > -    'postgres', "create table retain_test(a int);
> > -                                     select pg_switch_wal();
> > -                                     insert into retain_test values(1);
> > -                                     checkpoint;");
> > +$node_primary->advance_wal(1);
> > +$node_primary->safe_psql('postgres', "checkpoint;");
> > 
> > The original test generated some WAL after the segment switch, which
> > appears to be a significant characteristics of the test.
> 
> Still it does not matter for this specific case?  The logical slot has
> been already invalidated, so we don't care much about logical changes
> in WAL, do we?

The change itself doesn't seem to matter, but it seems intended to let
checkpoint trigger the removal of the last segment. However, I'm
unsure how the insert would influence this that way. If my
understanding is correct, then I'd support its removal.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
@ 2023-07-19 10:41     ` Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  1 sibling, 1 reply; 16+ messages in thread

From: Bharath Rupireddy @ 2023-07-19 10:41 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]; [email protected]; [email protected]

On Fri, Jun 16, 2023 at 8:00 AM Michael Paquier <[email protected]> wrote:
>
> On Thu, Jun 15, 2023 at 01:40:15PM +0900, Kyotaro Horiguchi wrote:
> > +                     "CREATE TABLE tt (); DROP TABLE tt; SELECT pg_switch_wal();");
> >
> > At least since 11, we can utilize pg_logical_emit_message() for this
> > purpose. It's more lightweight and seems appropriate, not only because
> > it doesn't cause any side effects but also bacause we don't have to
> > worry about name conflicts.
>
> Making this as cheap as possible by design is a good concept for a
> common routine.  +1.

While it seems reasonable to use pg_logical_emit_message, it won't
work for all the cases - what if someone wants to advance WAL by a few
WAL files? I think pg_switch_wal() is the way, no? For instance, the
replslot_limit.pl test increases the WAL in a very calculated way - it
increases by 5 WAL files. So, -1 to use pg_logical_emit_message.

I understand the naming conflicts for the table name used ('tt' in
this case). If the table name 'tt' looks so simple and easy for
someone to have tables with that name in their tests file, we can
generate a random table name in advance_wal, something like in the
attached v2 patch.

> > -              SELECT 'finished';",
> > -             timeout => $PostgreSQL::Test::Utils::timeout_default));
> > -is($result[1], 'finished', 'check if checkpoint command is not blocked');
> > -
> > +$node_primary2->advance_wal(1);
> > +$node_primary2->safe_psql('postgres', 'CHECKPOINT;');
> >
> > This test anticipates that the checkpoint could get blocked. Shouldn't
> > we keep the timeout?
>
> Indeed, this would partially invalidate what's getting tested in light
> of 1816a1c6 where we run a secondary command after the checkpoint.  So
> the last SELECT should remain around.

Changed.

> > -$node_primary->safe_psql(
> > -    'postgres', "create table retain_test(a int);
> > -                                     select pg_switch_wal();
> > -                                     insert into retain_test values(1);
> > -                                     checkpoint;");
> > +$node_primary->advance_wal(1);
> > +$node_primary->safe_psql('postgres', "checkpoint;");
> >
> > The original test generated some WAL after the segment switch, which
> > appears to be a significant characteristics of the test.
>
> Still it does not matter for this specific case?  The logical slot has
> been already invalidated, so we don't care much about logical changes
> in WAL, do we?

Correct, the slot has already been invalidated and the test is
verifying that WAL isn't retained by the invalidated slot, so
essentially what it needs is to generate "some" wal. So, using
advance_wal there seems fine to me. CFBot doesn't complain anything -
https://github.com/BRupireddy/postgres/tree/add_a_TAP_test_function_to_generate_WAL_v2.

Attached the v2 patch. Thoughts?

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com


Attachments:

  [application/x-patch] v2-0001-Add-a-TAP-test-function-to-generate-WAL.patch (7.4K, ../../CALj2ACUkXXsCTNoqhTr=S-CV7qdCB=QEiMvuuo31brLkLz4o6w@mail.gmail.com/2-v2-0001-Add-a-TAP-test-function-to-generate-WAL.patch)
  download | inline diff:
From 959bd5c7dd00be05c98ab5a68406ac37af61a52f Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Wed, 19 Jul 2023 09:58:11 +0000
Subject: [PATCH v2] Add a TAP test function to generate WAL

This commit adds a perl function in Cluster.pm to generate WAL.
Some TAP tests are now using their own way to generate WAL.
Generalizing this functionality enables multiple TAP tests to
reuse the functionality.
---
 src/test/perl/PostgreSQL/Test/Cluster.pm      | 24 ++++++++++
 src/test/recovery/t/001_stream_rep.pl         |  6 +--
 src/test/recovery/t/019_replslot_limit.pl     | 48 +++++--------------
 .../t/035_standby_logical_decoding.pl         |  7 +--
 4 files changed, 40 insertions(+), 45 deletions(-)

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 5e161dbee6..583c63f3db 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -3120,6 +3120,30 @@ sub create_logical_slot_on_standby
 
 =pod
 
+=item $node->advance_wal($n)
+
+Advance WAL of given node by $n segments
+
+=cut
+
+sub advance_wal
+{
+	my ($self, $n) = @_;
+
+	# Generate a random table name.
+	my $table_name = $self->safe_psql('postgres',
+			"SELECT substr(md5(random()::text), 1, 10)::text;");
+
+	# Advance by $n segments (= (wal_segment_size * $n) bytes).
+	for (my $i = 0; $i < $n; $i++)
+	{
+		$self->safe_psql('postgres',
+			qq{CREATE TABLE "$table_name"(); DROP TABLE "$table_name"; SELECT pg_switch_wal();});
+	}
+}
+
+=pod
+
 =back
 
 =cut
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index 0c72ba0944..0e256dab8d 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -496,11 +496,7 @@ $node_primary->safe_psql('postgres',
 my $segment_removed = $node_primary->safe_psql('postgres',
 	'SELECT pg_walfile_name(pg_current_wal_lsn())');
 chomp($segment_removed);
-$node_primary->psql(
-	'postgres', "
-	CREATE TABLE tab_phys_slot (a int);
-	INSERT INTO tab_phys_slot VALUES (generate_series(1,10));
-	SELECT pg_switch_wal();");
+$node_primary->advance_wal(1);
 my $current_lsn =
   $node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
 chomp($current_lsn);
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 33e50ad933..a312155250 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -59,7 +59,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved|t", 'check the catching-up state');
 
 # Advance WAL by five segments (= 5MB) on primary
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is always "safe" when fitting max_wal_size
@@ -69,7 +69,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved|t",
 	'check that it is safe if WAL fits in max_wal_size');
 
-advance_wal($node_primary, 4);
+$node_primary->advance_wal(4);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is always "safe" when max_slot_wal_keep_size is not set
@@ -100,7 +100,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved", 'check that max_slot_wal_keep_size is working');
 
 # Advance WAL again then checkpoint, reducing remain by 2 MB.
-advance_wal($node_primary, 2);
+$node_primary->advance_wal(2);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is still working
@@ -118,7 +118,7 @@ $node_standby->stop;
 $result = $node_primary->safe_psql('postgres',
 	"ALTER SYSTEM SET wal_keep_size to '8MB'; SELECT pg_reload_conf();");
 # Advance WAL again then checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
 $result = $node_primary->safe_psql('postgres',
 	"SELECT wal_status as remain FROM pg_replication_slots WHERE slot_name = 'rep1'"
 );
@@ -134,7 +134,7 @@ $node_primary->wait_for_catchup($node_standby);
 $node_standby->stop;
 
 # Advance WAL again without checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
 
 # Slot gets into 'reserved' state
 $result = $node_primary->safe_psql('postgres',
@@ -145,7 +145,7 @@ is($result, "extended", 'check that the slot state changes to "extended"');
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # Advance WAL again without checkpoint; remain goes to 0.
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
 
 # Slot gets into 'unreserved' state and safe_wal_size is negative
 $result = $node_primary->safe_psql('postgres',
@@ -174,7 +174,7 @@ $node_primary->safe_psql('postgres',
 
 # Advance WAL again. The slot loses the oldest segment by the next checkpoint
 my $logstart = get_log_size($node_primary);
-advance_wal($node_primary, 7);
+$node_primary->advance_wal(7);
 
 # Now create another checkpoint and wait until the WARNING is issued
 $node_primary->safe_psql('postgres',
@@ -275,18 +275,11 @@ $node_standby->init_from_backup($node_primary2, $backup_name,
 	has_streaming => 1);
 $node_standby->append_conf('postgresql.conf', "primary_slot_name = 'rep1'");
 $node_standby->start;
-my @result =
-  split(
-	'\n',
-	$node_primary2->safe_psql(
-		'postgres',
-		"CREATE TABLE tt();
-		 DROP TABLE tt;
-		 SELECT pg_switch_wal();
-		 CHECKPOINT;
-		 SELECT 'finished';",
-		timeout => $PostgreSQL::Test::Utils::timeout_default));
-is($result[1], 'finished', 'check if checkpoint command is not blocked');
+$node_primary2->advance_wal(1);
+$result = $node_primary2->safe_psql('postgres',
+			"CHECKPOINT; SELECT 'finished';",
+			timeout => $PostgreSQL::Test::Utils::timeout_default);
+is($result, 'finished', 'check if checkpoint command is not blocked');
 
 $node_primary2->stop;
 $node_standby->stop;
@@ -372,7 +365,7 @@ $logstart = get_log_size($node_primary3);
 # freeze walsender and walreceiver. Slot will still be active, but walreceiver
 # won't get anything anymore.
 kill 'STOP', $senderpid, $receiverpid;
-advance_wal($node_primary3, 2);
+$node_primary3->advance_wal(2);
 
 my $msg_logged = 0;
 my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
@@ -418,21 +411,6 @@ kill 'CONT', $receiverpid;
 $node_primary3->stop;
 $node_standby3->stop;
 
-#####################################
-# Advance WAL of $node by $n segments
-sub advance_wal
-{
-	my ($node, $n) = @_;
-
-	# Advance by $n segments (= (wal_segment_size * $n) bytes) on primary.
-	for (my $i = 0; $i < $n; $i++)
-	{
-		$node->safe_psql('postgres',
-			"CREATE TABLE t (); DROP TABLE t; SELECT pg_switch_wal();");
-	}
-	return;
-}
-
 # return the size of logfile of $node in bytes
 sub get_log_size
 {
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 480e6d6caa..2d19adab4e 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -524,11 +524,8 @@ my $walfile_name = $node_primary->safe_psql('postgres',
 chomp($walfile_name);
 
 # Generate some activity and switch WAL file on the primary
-$node_primary->safe_psql(
-	'postgres', "create table retain_test(a int);
-									 select pg_switch_wal();
-									 insert into retain_test values(1);
-									 checkpoint;");
+$node_primary->advance_wal(1);
+$node_primary->safe_psql('postgres', "checkpoint;");
 
 # Wait for the standby to catch up
 $node_primary->wait_for_replay_catchup($node_standby);
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
@ 2023-12-18 05:39       ` Bharath Rupireddy <[email protected]>
  2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Bharath Rupireddy @ 2023-12-18 05:39 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]; [email protected]; [email protected]

On Wed, Jul 19, 2023 at 4:11 PM Bharath Rupireddy
<[email protected]> wrote:
>
> Attached the v2 patch. Thoughts?

Rebase needed, attached v3 patch.

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com


Attachments:

  [application/octet-stream] v3-0001-Add-a-TAP-test-function-to-generate-WAL.patch (7.3K, ../../CALj2ACXR7FkdvzK1XaansV05awGnaCH82yu9M4gBiCueU_0GSA@mail.gmail.com/2-v3-0001-Add-a-TAP-test-function-to-generate-WAL.patch)
  download | inline diff:
From 823ca03dcbe715c43ceffdceee52269311950f95 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Mon, 18 Dec 2023 05:38:19 +0000
Subject: [PATCH v3] Add a TAP test function to generate WAL

This commit adds a perl function in Cluster.pm to generate WAL.
Some TAP tests are now using their own way to generate WAL.
Generalizing this functionality enables multiple TAP tests to
reuse the functionality.
---
 src/test/perl/PostgreSQL/Test/Cluster.pm      | 24 ++++++++++
 src/test/recovery/t/001_stream_rep.pl         |  6 +--
 src/test/recovery/t/019_replslot_limit.pl     | 48 +++++--------------
 .../t/035_standby_logical_decoding.pl         |  7 +--
 4 files changed, 40 insertions(+), 45 deletions(-)

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index a020377761..c0fea242c0 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -3178,6 +3178,30 @@ sub create_logical_slot_on_standby
 
 =pod
 
+=item $node->advance_wal($n)
+
+Advance WAL of given node by $n segments
+
+=cut
+
+sub advance_wal
+{
+	my ($self, $n) = @_;
+
+	# Generate a random table name.
+	my $table_name = $self->safe_psql('postgres',
+			"SELECT substr(md5(random()::text), 1, 10)::text;");
+
+	# Advance by $n segments (= (wal_segment_size * $n) bytes).
+	for (my $i = 0; $i < $n; $i++)
+	{
+		$self->safe_psql('postgres',
+			qq{CREATE TABLE "$table_name"(); DROP TABLE "$table_name"; SELECT pg_switch_wal();});
+	}
+}
+
+=pod
+
 =back
 
 =cut
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index 95f9b0d772..f0de921b4b 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -522,11 +522,7 @@ $node_primary->safe_psql('postgres',
 my $segment_removed = $node_primary->safe_psql('postgres',
 	'SELECT pg_walfile_name(pg_current_wal_lsn())');
 chomp($segment_removed);
-$node_primary->psql(
-	'postgres', "
-	CREATE TABLE tab_phys_slot (a int);
-	INSERT INTO tab_phys_slot VALUES (generate_series(1,10));
-	SELECT pg_switch_wal();");
+$node_primary->advance_wal(1);
 my $current_lsn =
   $node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
 chomp($current_lsn);
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 7d94f15778..e4b75c6545 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -59,7 +59,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved|t", 'check the catching-up state');
 
 # Advance WAL by five segments (= 5MB) on primary
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is always "safe" when fitting max_wal_size
@@ -69,7 +69,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved|t",
 	'check that it is safe if WAL fits in max_wal_size');
 
-advance_wal($node_primary, 4);
+$node_primary->advance_wal(4);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is always "safe" when max_slot_wal_keep_size is not set
@@ -100,7 +100,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved", 'check that max_slot_wal_keep_size is working');
 
 # Advance WAL again then checkpoint, reducing remain by 2 MB.
-advance_wal($node_primary, 2);
+$node_primary->advance_wal(2);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is still working
@@ -118,7 +118,7 @@ $node_standby->stop;
 $result = $node_primary->safe_psql('postgres',
 	"ALTER SYSTEM SET wal_keep_size to '8MB'; SELECT pg_reload_conf();");
 # Advance WAL again then checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
 $result = $node_primary->safe_psql('postgres',
 	"SELECT wal_status as remain FROM pg_replication_slots WHERE slot_name = 'rep1'"
 );
@@ -134,7 +134,7 @@ $node_primary->wait_for_catchup($node_standby);
 $node_standby->stop;
 
 # Advance WAL again without checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
 
 # Slot gets into 'reserved' state
 $result = $node_primary->safe_psql('postgres',
@@ -145,7 +145,7 @@ is($result, "extended", 'check that the slot state changes to "extended"');
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # Advance WAL again without checkpoint; remain goes to 0.
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
 
 # Slot gets into 'unreserved' state and safe_wal_size is negative
 $result = $node_primary->safe_psql('postgres',
@@ -174,7 +174,7 @@ $node_primary->safe_psql('postgres',
 
 # Advance WAL again. The slot loses the oldest segment by the next checkpoint
 my $logstart = -s $node_primary->logfile;
-advance_wal($node_primary, 7);
+$node_primary->advance_wal(7);
 
 # Now create another checkpoint and wait until the WARNING is issued
 $node_primary->safe_psql('postgres',
@@ -275,18 +275,11 @@ $node_standby->init_from_backup($node_primary2, $backup_name,
 	has_streaming => 1);
 $node_standby->append_conf('postgresql.conf', "primary_slot_name = 'rep1'");
 $node_standby->start;
-my @result =
-  split(
-	'\n',
-	$node_primary2->safe_psql(
-		'postgres',
-		"CREATE TABLE tt();
-		 DROP TABLE tt;
-		 SELECT pg_switch_wal();
-		 CHECKPOINT;
-		 SELECT 'finished';",
-		timeout => $PostgreSQL::Test::Utils::timeout_default));
-is($result[1], 'finished', 'check if checkpoint command is not blocked');
+$node_primary2->advance_wal(1);
+$result = $node_primary2->safe_psql('postgres',
+			"CHECKPOINT; SELECT 'finished';",
+			timeout => $PostgreSQL::Test::Utils::timeout_default);
+is($result, 'finished', 'check if checkpoint command is not blocked');
 
 $node_primary2->stop;
 $node_standby->stop;
@@ -372,7 +365,7 @@ $logstart = -s $node_primary3->logfile;
 # freeze walsender and walreceiver. Slot will still be active, but walreceiver
 # won't get anything anymore.
 kill 'STOP', $senderpid, $receiverpid;
-advance_wal($node_primary3, 2);
+$node_primary3->advance_wal(2);
 
 my $msg_logged = 0;
 my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
@@ -418,19 +411,4 @@ kill 'CONT', $receiverpid;
 $node_primary3->stop;
 $node_standby3->stop;
 
-#####################################
-# Advance WAL of $node by $n segments
-sub advance_wal
-{
-	my ($node, $n) = @_;
-
-	# Advance by $n segments (= (wal_segment_size * $n) bytes) on primary.
-	for (my $i = 0; $i < $n; $i++)
-	{
-		$node->safe_psql('postgres',
-			"CREATE TABLE t (); DROP TABLE t; SELECT pg_switch_wal();");
-	}
-	return;
-}
-
 done_testing();
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 9c34c0d36c..5d7c278d01 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -524,11 +524,8 @@ my $walfile_name = $node_primary->safe_psql('postgres',
 chomp($walfile_name);
 
 # Generate some activity and switch WAL file on the primary
-$node_primary->safe_psql(
-	'postgres', "create table retain_test(a int);
-									 select pg_switch_wal();
-									 insert into retain_test values(1);
-									 checkpoint;");
+$node_primary->advance_wal(1);
+$node_primary->safe_psql('postgres', "checkpoint;");
 
 # Wait for the standby to catch up
 $node_primary->wait_for_replay_catchup($node_standby);
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
@ 2023-12-18 11:48         ` Euler Taveira <[email protected]>
  2023-12-19 04:21           ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Euler Taveira @ 2023-12-18 11:48 UTC (permalink / raw)
  To: Bharath Rupireddy <[email protected]>; Michael Paquier <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

On Mon, Dec 18, 2023, at 2:39 AM, Bharath Rupireddy wrote:
> Rebase needed, attached v3 patch.

I think you don't understand the suggestion proposed by Michael and Kyotaro. If
you do a comparison with the following SQL commands:

euler=# select pg_walfile_name(pg_current_wal_lsn());
     pg_walfile_name      
--------------------------
000000010000000000000040
(1 row)

euler=# select pg_logical_emit_message(true, 'prefix', 'message4');
pg_logical_emit_message 
-------------------------
0/400000A8
(1 row)

euler=# select pg_switch_wal();
pg_switch_wal 
---------------
0/400000F0
(1 row)

euler=# create table cc (b int);
CREATE TABLE
euler=# drop table cc;
DROP TABLE
euler=# select pg_switch_wal();
pg_switch_wal 
---------------
0/41017C88
(1 row)

euler=# select pg_walfile_name(pg_current_wal_lsn());
     pg_walfile_name      
--------------------------
000000010000000000000041
(1 row)

You get

$ pg_waldump 000000010000000000000040
rmgr: Standby     len (rec/tot):     50/    50, tx:          0, lsn: 0/40000028, prev 0/3F0001C0, desc: RUNNING_XACTS nextXid 295180 latestCompletedXid 295179 oldestRunningXid 295180
rmgr: LogicalMessage len (rec/tot):     65/    65, tx:     295180, lsn: 0/40000060, prev 0/40000028, desc: MESSAGE transactional, prefix "prefix"; payload (8 bytes): 6D 65 73 73 61 67 65 34
rmgr: Transaction len (rec/tot):     46/    46, tx:     295180, lsn: 0/400000A8, prev 0/40000060, desc: COMMIT 2023-12-18 08:35:06.821322 -03
rmgr: XLOG        len (rec/tot):     24/    24, tx:          0, lsn: 0/400000D8, prev 0/400000A8, desc: SWITCH 

$ pg_waldump 000000010000000000000041
rmgr: Standby     len (rec/tot):     50/    50, tx:          0, lsn: 0/41000028, prev 0/400000D8, desc: RUNNING_XACTS nextXid 295181 latestCompletedXid 295180 oldestRunningXid 295181
rmgr: Storage     len (rec/tot):     42/    42, tx:          0, lsn: 0/41000060, prev 0/41000028, desc: CREATE base/33287/88102
rmgr: Heap2       len (rec/tot):     60/    60, tx:     295181, lsn: 0/41000090, prev 0/41000060, desc: NEW_CID rel: 1663/33287/1247, tid: 14/16, cmin: 0, cmax: 4294967295, combo: 4294967295
rmgr: Heap        len (rec/tot):     54/  3086, tx:     295181, lsn: 0/410000D0, prev 0/41000090, desc: INSERT off: 16, flags: 0x00, blkref #0: rel 1663/33287/1247 blk 14 FPW
rmgr: Btree       len (rec/tot):     53/  5133, tx:     295181, lsn: 0/41000CE0, prev 0/410000D0, desc: INSERT_LEAF off: 252, blkref #0: rel 1663/33287/2703 blk 2 FPW
.
.
.
rmgr: Btree       len (rec/tot):     72/    72, tx:     295181, lsn: 0/41016E48, prev 0/41014F00, desc: INSERT_LEAF off: 111, blkref #0: rel 1663/33287/2674 blk 7
rmgr: Heap2       len (rec/tot):     69/    69, tx:     295181, lsn: 0/41016E90, prev 0/41016E48, desc: PRUNE snapshotConflictHorizon: 295177, nredirected: 0, ndead: 7, nunused: 0, redirected: [], dead: [20, 21, 22, 23, 24, 26, 27], unused: [], blkref #0: rel 1663/33287/1249 blk 17
rmgr: Transaction len (rec/tot):    385/   385, tx:     295181, lsn: 0/41016ED8, prev 0/41016E90, desc: INVALIDATION ; inval msgs: catcache 80 catcache 79 catcache 80 catcache 79 catcache 55 catcache 54 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 snapshot 2608 relcache 88102
rmgr: Standby     len (rec/tot):     42/    42, tx:     295181, lsn: 0/41017060, prev 0/41016ED8, desc: LOCK xid 295181 db 33287 rel 88102 
rmgr: Transaction len (rec/tot):    405/   405, tx:     295181, lsn: 0/41017090, prev 0/41017060, desc: COMMIT 2023-12-18 08:35:22.342122 -03; inval msgs: catcache 80 catcache 79 catcache 80 catcache 79 catcache 55 catcache 54 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 snapshot 2608 relcache 88102
rmgr: Standby     len (rec/tot):     42/    42, tx:     295182, lsn: 0/41017228, prev 0/41017090, desc: LOCK xid 295182 db 33287 rel 88102 
rmgr: Heap2       len (rec/tot):     61/    61, tx:     295182, lsn: 0/41017258, prev 0/41017228, desc: PRUNE snapshotConflictHorizon: 295177, nredirected: 0, ndead: 3, nunused: 0, redirected: [], dead: [9, 12, 15], unused: [], blkref #0: rel 1663/33287/2608 blk 3
rmgr: Heap2       len (rec/tot):     60/    60, tx:     295182, lsn: 0/41017298, prev 0/41017258, desc: NEW_CID rel: 1663/33287/1247, tid: 14/17, cmin: 4294967295, cmax: 0, combo: 4294967295
rmgr: Heap        len (rec/tot):     54/    54, tx:     295182, lsn: 0/410172D8, prev 0/41017298, desc: DELETE xmax: 295182, off: 17, infobits: [KEYS_UPDATED], flags: 0x00, blkref #0: rel 1663/33287/1247 blk 14
.
.
.
rmgr: Heap2       len (rec/tot):     60/    60, tx:     295182, lsn: 0/410178D8, prev 0/410178A0, desc: NEW_CID rel: 1663/33287/2608, tid: 3/24, cmin: 4294967295, cmax: 2, combo: 4294967295
rmgr: Heap        len (rec/tot):     54/    54, tx:     295182, lsn: 0/41017918, prev 0/410178D8, desc: DELETE xmax: 295182, off: 24, infobits: [KEYS_UPDATED], flags: 0x00, blkref #0: rel 1663/33287/2608 blk 3
rmgr: Transaction len (rec/tot):    321/   321, tx:     295182, lsn: 0/41017950, prev 0/41017918, desc: INVALIDATION ; inval msgs: catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 55 catcache 54 relcache 88102 snapshot 2608
rmgr: Transaction len (rec/tot):    469/   469, tx:     295182, lsn: 0/41017A98, prev 0/41017950, desc: COMMIT 2023-12-18 08:35:25.053905 -03; rels: base/33287/88102; dropped stats: 2/33287/88102; inval msgs: catcache 80 catcache 79 catcache 80 catcache 79 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 7 catcache 6 catcache 55 catcache 54 snapshot 2608 snapshot 2608 relcache 88102 snapshot 2608
rmgr: XLOG        len (rec/tot):     24/    24, tx:          0, lsn: 0/41017C70, prev 0/41017A98, desc: SWITCH

The difference is

euler=# select '0/400000A8'::pg_lsn - '0/40000028'::pg_lsn;
?column? 
----------
      128
(1 row)

euler=# select '0/41017A98'::pg_lsn - '0/41000028'::pg_lsn;
?column? 
----------
    96880
(1 row)


It is cheaper.


--
Euler Taveira
EDB   https://www.enterprisedb.com/


^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
@ 2023-12-19 04:21           ` Michael Paquier <[email protected]>
  2023-12-19 05:55             ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Michael Paquier @ 2023-12-19 04:21 UTC (permalink / raw)
  To: Euler Taveira <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; Kyotaro Horiguchi <[email protected]>; Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

On Mon, Dec 18, 2023 at 08:48:09AM -0300, Euler Taveira wrote:
> It is cheaper.

Agreed that this could just use a set of pg_logical_emit_message()
when jumping across N segments.  Another thing that seems quite
important to me is to force a flush of WAL with the last segment
switch, and the new "flush" option of pg_logical_emit_message() can
be very handy for this purpose.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-19 04:21           ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
@ 2023-12-19 05:55             ` Bharath Rupireddy <[email protected]>
  2023-12-19 23:00               ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Bharath Rupireddy @ 2023-12-19 05:55 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Euler Taveira <[email protected]>; Kyotaro Horiguchi <[email protected]>; Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

On Tue, Dec 19, 2023 at 9:51 AM Michael Paquier <[email protected]> wrote:
>
> On Mon, Dec 18, 2023 at 08:48:09AM -0300, Euler Taveira wrote:
> > It is cheaper.
>
> Agreed that this could just use a set of pg_logical_emit_message()
> when jumping across N segments.

Thanks. I missed the point of using pg_logical_emit_message() over
CREATE .. DROP TABLE to generate WAL. And, I agree that it's better
and relatively cheaper in terms of amount of WAL generated.

> Another thing that seems quite
> important to me is to force a flush of WAL with the last segment
> switch, and the new "flush" option of pg_logical_emit_message() can
> be very handy for this purpose.

I used pg_logical_emit_message() in non-transactional mode without
needing an explicit WAL flush as the pg_switch_wal() does a WAL flush
at the end [1].

Attached v4 patch.

[1]
    /*
     * If this was an XLOG_SWITCH record, flush the record and the empty
     * padding space that fills the rest of the segment, and perform
     * end-of-segment actions (eg, notifying archiver).
     */
    if (class == WALINSERT_SPECIAL_SWITCH)
    {
        TRACE_POSTGRESQL_WAL_SWITCH();
        XLogFlush(EndPos);

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com


Attachments:

  [application/x-patch] v4-0001-Add-a-TAP-test-function-to-generate-WAL.patch (7.2K, ../../CALj2ACXppTLS1pKa=M-h-VxWHSTo4iJvHYik_J+pLQxdeLMAzQ@mail.gmail.com/2-v4-0001-Add-a-TAP-test-function-to-generate-WAL.patch)
  download | inline diff:
From b7fa7545eb983aaf92e3d7e99bdf76ef42b8e40e Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Tue, 19 Dec 2023 05:49:20 +0000
Subject: [PATCH v4] Add a TAP test function to generate WAL

This commit adds a perl function in Cluster.pm to generate WAL.
Some TAP tests are now using their own way to generate WAL.
Generalizing this functionality enables multiple TAP tests to
reuse the functionality.
---
 src/test/perl/PostgreSQL/Test/Cluster.pm      | 22 +++++++++
 src/test/recovery/t/001_stream_rep.pl         |  6 +--
 src/test/recovery/t/019_replslot_limit.pl     | 48 +++++--------------
 .../t/035_standby_logical_decoding.pl         |  7 +--
 4 files changed, 38 insertions(+), 45 deletions(-)

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index a020377761..ad575ed6d6 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -3178,6 +3178,28 @@ sub create_logical_slot_on_standby
 
 =pod
 
+=item $node->advance_wal($n)
+
+Advance WAL of given node by $n segments
+
+=cut
+
+sub advance_wal
+{
+	my ($self, $n) = @_;
+
+	# Advance by $n segments (= (wal_segment_size * $n) bytes).
+	for (my $i = 0; $i < $n; $i++)
+	{
+		$self->safe_psql('postgres', qq{
+			SELECT pg_logical_emit_message(false, '', 'foo');
+			SELECT pg_switch_wal();
+			});
+	}
+}
+
+=pod
+
 =back
 
 =cut
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index 95f9b0d772..f0de921b4b 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -522,11 +522,7 @@ $node_primary->safe_psql('postgres',
 my $segment_removed = $node_primary->safe_psql('postgres',
 	'SELECT pg_walfile_name(pg_current_wal_lsn())');
 chomp($segment_removed);
-$node_primary->psql(
-	'postgres', "
-	CREATE TABLE tab_phys_slot (a int);
-	INSERT INTO tab_phys_slot VALUES (generate_series(1,10));
-	SELECT pg_switch_wal();");
+$node_primary->advance_wal(1);
 my $current_lsn =
   $node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
 chomp($current_lsn);
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 7d94f15778..e4b75c6545 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -59,7 +59,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved|t", 'check the catching-up state');
 
 # Advance WAL by five segments (= 5MB) on primary
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is always "safe" when fitting max_wal_size
@@ -69,7 +69,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved|t",
 	'check that it is safe if WAL fits in max_wal_size');
 
-advance_wal($node_primary, 4);
+$node_primary->advance_wal(4);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is always "safe" when max_slot_wal_keep_size is not set
@@ -100,7 +100,7 @@ $result = $node_primary->safe_psql('postgres',
 is($result, "reserved", 'check that max_slot_wal_keep_size is working');
 
 # Advance WAL again then checkpoint, reducing remain by 2 MB.
-advance_wal($node_primary, 2);
+$node_primary->advance_wal(2);
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # The slot is still working
@@ -118,7 +118,7 @@ $node_standby->stop;
 $result = $node_primary->safe_psql('postgres',
 	"ALTER SYSTEM SET wal_keep_size to '8MB'; SELECT pg_reload_conf();");
 # Advance WAL again then checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
 $result = $node_primary->safe_psql('postgres',
 	"SELECT wal_status as remain FROM pg_replication_slots WHERE slot_name = 'rep1'"
 );
@@ -134,7 +134,7 @@ $node_primary->wait_for_catchup($node_standby);
 $node_standby->stop;
 
 # Advance WAL again without checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
 
 # Slot gets into 'reserved' state
 $result = $node_primary->safe_psql('postgres',
@@ -145,7 +145,7 @@ is($result, "extended", 'check that the slot state changes to "extended"');
 $node_primary->safe_psql('postgres', "CHECKPOINT;");
 
 # Advance WAL again without checkpoint; remain goes to 0.
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
 
 # Slot gets into 'unreserved' state and safe_wal_size is negative
 $result = $node_primary->safe_psql('postgres',
@@ -174,7 +174,7 @@ $node_primary->safe_psql('postgres',
 
 # Advance WAL again. The slot loses the oldest segment by the next checkpoint
 my $logstart = -s $node_primary->logfile;
-advance_wal($node_primary, 7);
+$node_primary->advance_wal(7);
 
 # Now create another checkpoint and wait until the WARNING is issued
 $node_primary->safe_psql('postgres',
@@ -275,18 +275,11 @@ $node_standby->init_from_backup($node_primary2, $backup_name,
 	has_streaming => 1);
 $node_standby->append_conf('postgresql.conf', "primary_slot_name = 'rep1'");
 $node_standby->start;
-my @result =
-  split(
-	'\n',
-	$node_primary2->safe_psql(
-		'postgres',
-		"CREATE TABLE tt();
-		 DROP TABLE tt;
-		 SELECT pg_switch_wal();
-		 CHECKPOINT;
-		 SELECT 'finished';",
-		timeout => $PostgreSQL::Test::Utils::timeout_default));
-is($result[1], 'finished', 'check if checkpoint command is not blocked');
+$node_primary2->advance_wal(1);
+$result = $node_primary2->safe_psql('postgres',
+			"CHECKPOINT; SELECT 'finished';",
+			timeout => $PostgreSQL::Test::Utils::timeout_default);
+is($result, 'finished', 'check if checkpoint command is not blocked');
 
 $node_primary2->stop;
 $node_standby->stop;
@@ -372,7 +365,7 @@ $logstart = -s $node_primary3->logfile;
 # freeze walsender and walreceiver. Slot will still be active, but walreceiver
 # won't get anything anymore.
 kill 'STOP', $senderpid, $receiverpid;
-advance_wal($node_primary3, 2);
+$node_primary3->advance_wal(2);
 
 my $msg_logged = 0;
 my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
@@ -418,19 +411,4 @@ kill 'CONT', $receiverpid;
 $node_primary3->stop;
 $node_standby3->stop;
 
-#####################################
-# Advance WAL of $node by $n segments
-sub advance_wal
-{
-	my ($node, $n) = @_;
-
-	# Advance by $n segments (= (wal_segment_size * $n) bytes) on primary.
-	for (my $i = 0; $i < $n; $i++)
-	{
-		$node->safe_psql('postgres',
-			"CREATE TABLE t (); DROP TABLE t; SELECT pg_switch_wal();");
-	}
-	return;
-}
-
 done_testing();
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 9c34c0d36c..5d7c278d01 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -524,11 +524,8 @@ my $walfile_name = $node_primary->safe_psql('postgres',
 chomp($walfile_name);
 
 # Generate some activity and switch WAL file on the primary
-$node_primary->safe_psql(
-	'postgres', "create table retain_test(a int);
-									 select pg_switch_wal();
-									 insert into retain_test values(1);
-									 checkpoint;");
+$node_primary->advance_wal(1);
+$node_primary->safe_psql('postgres', "checkpoint;");
 
 # Wait for the standby to catch up
 $node_primary->wait_for_replay_catchup($node_standby);
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-19 04:21           ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-19 05:55             ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
@ 2023-12-19 23:00               ` Michael Paquier <[email protected]>
  2023-12-20 03:24                 ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Michael Paquier @ 2023-12-19 23:00 UTC (permalink / raw)
  To: Bharath Rupireddy <[email protected]>; +Cc: Euler Taveira <[email protected]>; Kyotaro Horiguchi <[email protected]>; Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

On Tue, Dec 19, 2023 at 11:25:50AM +0530, Bharath Rupireddy wrote:
> I used pg_logical_emit_message() in non-transactional mode without
> needing an explicit WAL flush as the pg_switch_wal() does a WAL flush
> at the end [1].

Indeed, that should be enough to answer my comment.

> Attached v4 patch.

LGTM, thanks.  Euler, what do you think?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-19 04:21           ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-19 05:55             ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-19 23:00               ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
@ 2023-12-20 03:24                 ` Euler Taveira <[email protected]>
  2023-12-21 01:21                   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Euler Taveira @ 2023-12-20 03:24 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; Bharath Rupireddy <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

On Tue, Dec 19, 2023, at 8:00 PM, Michael Paquier wrote:
> On Tue, Dec 19, 2023 at 11:25:50AM +0530, Bharath Rupireddy wrote:
> > I used pg_logical_emit_message() in non-transactional mode without
> > needing an explicit WAL flush as the pg_switch_wal() does a WAL flush
> > at the end [1].
> 
> Indeed, that should be enough to answer my comment.
> 
> > Attached v4 patch.
> 
> LGTM, thanks.  Euler, what do you think?
> 

LGTM.


--
Euler Taveira
EDB   https://www.enterprisedb.com/


^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-19 04:21           ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-19 05:55             ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-19 23:00               ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-20 03:24                 ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
@ 2023-12-21 01:21                   ` Michael Paquier <[email protected]>
  2024-01-03 23:39                     ` Re: Add a perl function in Cluster.pm to generate WAL Tom Lane <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Michael Paquier @ 2023-12-21 01:21 UTC (permalink / raw)
  To: Euler Taveira <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; Kyotaro Horiguchi <[email protected]>; Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

On Wed, Dec 20, 2023 at 12:24:04AM -0300, Euler Taveira wrote:
> LGTM.

I was eyeing at 020_messages.pl and it has a pg_switch_wal() after a
transaction rollbacked.  020_archive_status.pl creates a table, does 
one segment switch, then a checkpoint (table is used afterwards).
Perhaps these could be changed with the new routine, but it does not
seem like this improves the readability of the tests, either, contrary
to the ones updated here where a fake table is created to force some
records.  What do you think?

We have a few more pg_switch_wal() calls, as well, but these rely on
WAL being already generated beforehand.

I have added a comment about pg_logical_emit_message() being in
non-transactional mode and the flush implied by pg_switch_wal() as
that's important, edited a bit the whole, then applied the patch.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-19 04:21           ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-19 05:55             ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-19 23:00               ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-20 03:24                 ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-21 01:21                   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
@ 2024-01-03 23:39                     ` Tom Lane <[email protected]>
  2024-01-04 13:00                       ` Re: Add a perl function in Cluster.pm to generate WAL Alexander Lakhin <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Tom Lane @ 2024-01-03 23:39 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Euler Taveira <[email protected]>; Bharath Rupireddy <[email protected]>; Kyotaro Horiguchi <[email protected]>; Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

Michael Paquier <[email protected]> writes:
> I have added a comment about pg_logical_emit_message() being in
> non-transactional mode and the flush implied by pg_switch_wal() as
> that's important, edited a bit the whole, then applied the patch.

Buildfarm member skink has failed 3 times in
035_standby_logical_decoding.pl in the last couple of days:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2024-01-03%2020%3A07%3A15

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2024-01-03%2017%3A09%3A27

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2024-01-01%2020%3A10%3A18

These all look like

# poll_query_until timed out executing this query:
# select (confl_active_logicalslot = 1) from pg_stat_database_conflicts where datname = 'testdb'
# expecting this output:
# t
# last actual query output:
# f

although it's notable that two different tests are involved
(vacuum vs. vacuum full).

I am not real sure what is happening there, but I see that c161ab74f
changed some details of how that test works, so I wonder if it's
responsible for these failures.  The timing isn't a perfect match,
since this commit went in two weeks ago, but I don't see any
more-recent commits that seem like plausible explanations.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-19 04:21           ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-19 05:55             ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-19 23:00               ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-20 03:24                 ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-21 01:21                   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2024-01-03 23:39                     ` Re: Add a perl function in Cluster.pm to generate WAL Tom Lane <[email protected]>
@ 2024-01-04 13:00                       ` Alexander Lakhin <[email protected]>
  2024-01-04 23:48                         ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Alexander Lakhin @ 2024-01-04 13:00 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Michael Paquier <[email protected]>; +Cc: Euler Taveira <[email protected]>; Bharath Rupireddy <[email protected]>; Kyotaro Horiguchi <[email protected]>; Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

Hello Tom,

04.01.2024 02:39, Tom Lane wrote:
> Buildfarm member skink has failed 3 times in
> 035_standby_logical_decoding.pl in the last couple of days:
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2024-01-03%2020%3A07%3A15
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2024-01-03%2017%3A09%3A27
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2024-01-01%2020%3A10%3A18
>
> These all look like
>
> # poll_query_until timed out executing this query:
> # select (confl_active_logicalslot = 1) from pg_stat_database_conflicts where datname = 'testdb'
> # expecting this output:
> # t
> # last actual query output:
> # f
>
> although it's notable that two different tests are involved
> (vacuum vs. vacuum full).
>
> I am not real sure what is happening there, but I see that c161ab74f
> changed some details of how that test works, so I wonder if it's
> responsible for these failures.  The timing isn't a perfect match,
> since this commit went in two weeks ago, but I don't see any
> more-recent commits that seem like plausible explanations.

Reproduced here.
As I can see in the failure logs you referenced, the first problem is:
#   Failed test 'inactiveslot slot invalidation is logged with vacuum on pg_authid'
#   at t/035_standby_logical_decoding.pl line 224.

It reminded me of:
https://www.postgresql.org/message-id/b2a1f7d0-7629-72c0-2da7-e9c4e336b010%40gmail.com

It seems that it's not something new, and maybe that my analysis is still
valid. If so, VACUUM FREEZE/autovacuum = off should fix the issue.

Best regards,
Alexander






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Add a perl function in Cluster.pm to generate WAL
  2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
  2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-19 04:21           ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-19 05:55             ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
  2023-12-19 23:00               ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2023-12-20 03:24                 ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
  2023-12-21 01:21                   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
  2024-01-03 23:39                     ` Re: Add a perl function in Cluster.pm to generate WAL Tom Lane <[email protected]>
  2024-01-04 13:00                       ` Re: Add a perl function in Cluster.pm to generate WAL Alexander Lakhin <[email protected]>
@ 2024-01-04 23:48                         ` Michael Paquier <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Michael Paquier @ 2024-01-04 23:48 UTC (permalink / raw)
  To: Alexander Lakhin <[email protected]>; +Cc: Tom Lane <[email protected]>; Euler Taveira <[email protected]>; Bharath Rupireddy <[email protected]>; Kyotaro Horiguchi <[email protected]>; Alvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]

On Thu, Jan 04, 2024 at 04:00:01PM +0300, Alexander Lakhin wrote:
> Reproduced here.

Did you just make the run slow enough to show the failure with
valgrind?

> As I can see in the failure logs you referenced, the first problem is:
> #   Failed test 'inactiveslot slot invalidation is logged with vacuum on pg_authid'
> #   at t/035_standby_logical_decoding.pl line 224.
> It reminded me of:
> https://www.postgresql.org/message-id/b2a1f7d0-7629-72c0-2da7-e9c4e336b010%40gmail.com
> 
> It seems that it's not something new, and maybe that my analysis is still
> valid. If so, VACUUM FREEZE/autovacuum = off should fix the issue.

Not sure about that yet.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 16+ messages in thread


end of thread, other threads:[~2024-01-04 23:48 UTC | newest]

Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18 16:19 [PATCH v38] Subscripting for jsonb Dmitrii Dolgov <[email protected]>
2023-06-11 01:44 Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
2023-06-15 04:40 ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
2023-06-16 02:30   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
2023-06-16 04:30     ` Re: Add a perl function in Cluster.pm to generate WAL Kyotaro Horiguchi <[email protected]>
2023-07-19 10:41     ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
2023-12-18 05:39       ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
2023-12-18 11:48         ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
2023-12-19 04:21           ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
2023-12-19 05:55             ` Re: Add a perl function in Cluster.pm to generate WAL Bharath Rupireddy <[email protected]>
2023-12-19 23:00               ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
2023-12-20 03:24                 ` Re: Add a perl function in Cluster.pm to generate WAL Euler Taveira <[email protected]>
2023-12-21 01:21                   ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[email protected]>
2024-01-03 23:39                     ` Re: Add a perl function in Cluster.pm to generate WAL Tom Lane <[email protected]>
2024-01-04 13:00                       ` Re: Add a perl function in Cluster.pm to generate WAL Alexander Lakhin <[email protected]>
2024-01-04 23:48                         ` Re: Add a perl function in Cluster.pm to generate WAL Michael Paquier <[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