public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v41 1/2] Subscripting for jsonb
2+ messages / 2 participants
[nested] [flat]

* [PATCH v41 1/2] Subscripting for jsonb
@ 2020-12-18 16:19 Dmitrii Dolgov <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Dmitrii Dolgov @ 2020-12-18 16:19 UTC (permalink / raw)

Subscripting implementation for jsonb. It does not support slices, does
not have a limit for number of subscripts and for assignment expects a
replace value to be of jsonb type. There is also one functional
difference in assignment via subscripting from jsonb_set, when an
original jsonb container is NULL, subscripting replaces it with an empty
jsonb and proceed with assignment.

For the sake of code reuse, some parts of jsonb functionality were
rearranged to allow use the same functions for jsonb_set and assign
subscripting operation.

The original idea belongs to Oleg Bartunov.

Reviewed-by: Tom Lane, Arthur Zakirov, Pavel Stehule
---
 doc/src/sgml/json.sgml              |  48 ++++
 src/backend/utils/adt/Makefile      |   1 +
 src/backend/utils/adt/jsonb_util.c  |  76 ++++-
 src/backend/utils/adt/jsonbsubs.c   | 413 ++++++++++++++++++++++++++++
 src/backend/utils/adt/jsonfuncs.c   | 180 ++++++------
 src/include/catalog/pg_proc.dat     |   4 +
 src/include/catalog/pg_type.dat     |   3 +-
 src/include/utils/jsonb.h           |   6 +-
 src/test/regress/expected/jsonb.out | 272 +++++++++++++++++-
 src/test/regress/sql/jsonb.sql      |  84 +++++-
 10 files changed, 982 insertions(+), 105 deletions(-)
 create mode 100644 src/backend/utils/adt/jsonbsubs.c

diff --git a/doc/src/sgml/json.sgml b/doc/src/sgml/json.sgml
index 5b9a5557a4..100d1a60f4 100644
--- a/doc/src/sgml/json.sgml
+++ b/doc/src/sgml/json.sgml
@@ -602,6 +602,54 @@ SELECT jdoc-&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]
+UPDATE table_name SET jsonb_field[0] = '1';
+</programlisting>
+
+  </para>
+ </sect2>
+
  <sect2>
   <title>Transforms</title>
 
diff --git a/src/backend/utils/adt/Makefile b/src/backend/utils/adt/Makefile
index 82732146d3..279ff15ade 100644
--- a/src/backend/utils/adt/Makefile
+++ b/src/backend/utils/adt/Makefile
@@ -50,6 +50,7 @@ OBJS = \
 	jsonb_op.o \
 	jsonb_util.o \
 	jsonfuncs.o \
+	jsonbsubs.o \
 	jsonpath.o \
 	jsonpath_exec.o \
 	jsonpath_gram.o \
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index 4eeffa1424..41a1c1f9bb 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -68,18 +68,29 @@ static JsonbValue *pushJsonbValueScalar(JsonbParseState **pstate,
 										JsonbIteratorToken seq,
 										JsonbValue *scalarVal);
 
+JsonbValue *
+JsonbToJsonbValue(Jsonb *jsonb)
+{
+	JsonbValue *val = (JsonbValue *) palloc(sizeof(JsonbValue));
+
+	val->type = jbvBinary;
+	val->val.binary.data = &jsonb->root;
+	val->val.binary.len = VARSIZE(jsonb) - VARHDRSZ;
+
+	return val;
+}
+
 /*
  * Turn an in-memory JsonbValue into a Jsonb for on-disk storage.
  *
- * There isn't a JsonbToJsonbValue(), because generally we find it more
- * convenient to directly iterate through the Jsonb representation and only
- * really convert nested scalar values.  JsonbIteratorNext() does this, so that
- * clients of the iteration code don't have to directly deal with the binary
- * representation (JsonbDeepContains() is a notable exception, although all
- * exceptions are internal to this module).  In general, functions that accept
- * a JsonbValue argument are concerned with the manipulation of scalar values,
- * or simple containers of scalar values, where it would be inconvenient to
- * deal with a great amount of other state.
+ * Generally we find it more convenient to directly iterate through the Jsonb
+ * representation and only really convert nested scalar values.
+ * JsonbIteratorNext() does this, so that clients of the iteration code don't
+ * have to directly deal with the binary representation (JsonbDeepContains() is
+ * a notable exception, although all exceptions are internal to this module).
+ * In general, functions that accept a JsonbValue argument are concerned with
+ * the manipulation of scalar values, or simple containers of scalar values,
+ * where it would be inconvenient to deal with a great amount of other state.
  */
 Jsonb *
 JsonbValueToJsonb(JsonbValue *val)
@@ -563,6 +574,30 @@ pushJsonbValue(JsonbParseState **pstate, JsonbIteratorToken seq,
 	JsonbValue *res = NULL;
 	JsonbValue	v;
 	JsonbIteratorToken tok;
+	int	i;
+
+	if (jbval && (seq == WJB_ELEM || seq == WJB_VALUE) && jbval->type == jbvObject)
+	{
+		pushJsonbValue(pstate, WJB_BEGIN_OBJECT, NULL);
+		for (i = 0; i < jbval->val.object.nPairs; i++)
+		{
+			pushJsonbValue(pstate, WJB_KEY, &jbval->val.object.pairs[i].key);
+			pushJsonbValue(pstate, WJB_VALUE, &jbval->val.object.pairs[i].value);
+		}
+
+		return pushJsonbValue(pstate, WJB_END_OBJECT, NULL);
+	}
+
+	if (jbval && (seq == WJB_ELEM || seq == WJB_VALUE) && jbval->type == jbvArray)
+	{
+		pushJsonbValue(pstate, WJB_BEGIN_ARRAY, NULL);
+		for (i = 0; i < jbval->val.array.nElems; i++)
+		{
+			pushJsonbValue(pstate, WJB_ELEM, &jbval->val.array.elems[i]);
+		}
+
+		return pushJsonbValue(pstate, WJB_END_ARRAY, NULL);
+	}
 
 	if (!jbval || (seq != WJB_ELEM && seq != WJB_VALUE) ||
 		jbval->type != jbvBinary)
@@ -573,9 +608,30 @@ pushJsonbValue(JsonbParseState **pstate, JsonbIteratorToken seq,
 
 	/* unpack the binary and add each piece to the pstate */
 	it = JsonbIteratorInit(jbval->val.binary.data);
+
+	if ((jbval->val.binary.data->header & JB_FSCALAR) && *pstate)
+	{
+		tok = JsonbIteratorNext(&it, &v, true);
+		Assert(tok == WJB_BEGIN_ARRAY);
+		Assert(v.type == jbvArray && v.val.array.rawScalar);
+
+		tok = JsonbIteratorNext(&it, &v, true);
+		Assert(tok == WJB_ELEM);
+
+		res = pushJsonbValueScalar(pstate, seq, &v);
+
+		tok = JsonbIteratorNext(&it, &v, true);
+		Assert(tok == WJB_END_ARRAY);
+		Assert(it == NULL);
+
+		return res;
+	}
+
 	while ((tok = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
 		res = pushJsonbValueScalar(pstate, tok,
-								   tok < WJB_BEGIN_ARRAY ? &v : NULL);
+								   tok < WJB_BEGIN_ARRAY ||
+								   (tok == WJB_BEGIN_ARRAY &&
+									v.val.array.rawScalar) ? &v : NULL);
 
 	return res;
 }
diff --git a/src/backend/utils/adt/jsonbsubs.c b/src/backend/utils/adt/jsonbsubs.c
new file mode 100644
index 0000000000..306c37b5a6
--- /dev/null
+++ b/src/backend/utils/adt/jsonbsubs.c
@@ -0,0 +1,413 @@
+/*-------------------------------------------------------------------------
+ *
+ * jsonbsubs.c
+ *	  Subscripting support functions for jsonb.
+ *
+ * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/utils/adt/jsonbsubs.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "executor/execExpr.h"
+#include "nodes/makefuncs.h"
+#include "nodes/nodeFuncs.h"
+#include "nodes/subscripting.h"
+#include "parser/parse_coerce.h"
+#include "parser/parse_expr.h"
+#include "utils/jsonb.h"
+#include "utils/jsonfuncs.h"
+#include "utils/builtins.h"
+#include "utils/lsyscache.h"
+
+
+/* SubscriptingRefState.workspace for jsonb subscripting execution */
+typedef struct JsonbSubWorkspace
+{
+	bool		expectArray;	/* jsonb root is expected to be an array */
+	Oid		   *indexOid;		/* OID of coerced subscript expression,
+								   could be only integer or text */
+	Datum	   *index;			/* Subscript values in Datum format */
+} JsonbSubWorkspace;
+
+
+/*
+ * Finish parse analysis of a SubscriptingRef expression for a jsonb.
+ *
+ * Transform the subscript expressions, coerce them to text,
+ * and determine the result type of the SubscriptingRef node.
+ */
+static void
+jsonb_subscript_transform(SubscriptingRef *sbsref,
+						  List *indirection,
+						  ParseState *pstate,
+						  bool isSlice,
+						  bool isAssignment)
+{
+	List	   *upperIndexpr = NIL;
+	ListCell   *idx;
+
+	/*
+	 * Transform and convert the subscript expressions. Jsonb subscripting does
+	 * not support slices, look only and the upper index.
+	 */
+	foreach(idx, indirection)
+	{
+		A_Indices  *ai = lfirst_node(A_Indices, idx);
+		Node	   *subExpr;
+
+		if (isSlice)
+		{
+			Node	*expr = ai->uidx ? ai->uidx : ai->lidx;
+
+			ereport(ERROR,
+					(errcode(ERRCODE_DATATYPE_MISMATCH),
+					 errmsg("jsonb subscript does not support slices"),
+					 parser_errposition(pstate, exprLocation(expr))));
+		}
+
+		if (ai->uidx)
+		{
+			Oid subExprType = InvalidOid,
+				targetType = UNKNOWNOID;
+
+			subExpr = transformExpr(pstate, ai->uidx, pstate->p_expr_kind);
+			subExprType = exprType(subExpr);
+
+			if (subExprType != UNKNOWNOID)
+			{
+				Oid 	targets[2] = {INT4OID, TEXTOID};
+
+				/*
+				 * Jsonb can handle multiple subscript types, but cases when a
+				 * subscript could be coerced to multiple target types must be
+				 * avoided, similar to overloaded functions. It could be
+				 * possibly extend with jsonpath in the future.
+				 */
+				for (int i = 0; i < 2; i++)
+				{
+					if (can_coerce_type(1, &subExprType, &targets[i], COERCION_IMPLICIT))
+					{
+						/*
+						 * One type has already succeeded, it means there are
+						 * two coercion targets possible, failure.
+						 */
+						if (targetType != UNKNOWNOID)
+							ereport(ERROR,
+									(errcode(ERRCODE_DATATYPE_MISMATCH),
+									 errmsg("subscript type is not supported"),
+									 errhint("Jsonb subscript must be coerced "
+											 "only to one type, integer or text."),
+									 parser_errposition(pstate, exprLocation(subExpr))));
+
+						targetType = targets[i];
+					}
+				}
+
+				/*
+				 * No suitable types were found, failure.
+				 */
+				if (targetType == UNKNOWNOID)
+					ereport(ERROR,
+							(errcode(ERRCODE_DATATYPE_MISMATCH),
+							 errmsg("subscript type is not supported"),
+							 errhint("Jsonb subscript must be coercet to either integer or text"),
+							 parser_errposition(pstate, exprLocation(subExpr))));
+			}
+			else
+				targetType = TEXTOID;
+
+			/*
+			 * We known from can_coerce_type that coercion will succeed, so
+			 * coerce_type could be used. Note the implicit coercion context,
+			 * which is required to handle subscripts of different types,
+			 * similar to overloaded functions.
+			 */
+			subExpr = coerce_type(pstate,
+								  subExpr, subExprType,
+								  targetType, -1,
+								  COERCION_IMPLICIT,
+								  COERCE_IMPLICIT_CAST,
+								  -1);
+			if (subExpr == NULL)
+				ereport(ERROR,
+						(errcode(ERRCODE_DATATYPE_MISMATCH),
+						 errmsg("jsonb subscript must have text type"),
+						 parser_errposition(pstate, exprLocation(subExpr))));
+		}
+		else
+		{
+			/*
+			 * Slice with omitted upper bound. Should not happen as we already
+			 * errored out on slice earlier, but handle this just in case.
+			 */
+			Assert(isSlice && ai->is_slice);
+			ereport(ERROR,
+					(errcode(ERRCODE_DATATYPE_MISMATCH),
+					 errmsg("jsonb subscript does not support slices"),
+					 parser_errposition(pstate, exprLocation(ai->uidx))));
+		}
+
+		upperIndexpr = lappend(upperIndexpr, subExpr);
+	}
+
+	/* store the transformed lists into the SubscriptRef node */
+	sbsref->refupperindexpr = upperIndexpr;
+	sbsref->reflowerindexpr = NIL;
+
+	/* Determine the result type of the subscripting operation; always jsonb */
+	sbsref->refrestype = JSONBOID;
+	sbsref->reftypmod = -1;
+}
+
+/*
+ * During execution, process the subscripts in a SubscriptingRef expression.
+ *
+ * The subscript expressions are already evaluated in Datum form in the
+ * SubscriptingRefState's arrays.  Check and convert them as necessary.
+ *
+ * If any subscript is NULL, we throw error in assignment cases, or in fetch
+ * cases set result to NULL and return false (instructing caller to skip the
+ * rest of the SubscriptingRef sequence).
+ */
+static bool
+jsonb_subscript_check_subscripts(ExprState *state,
+								 ExprEvalStep *op,
+								 ExprContext *econtext)
+{
+	SubscriptingRefState *sbsrefstate = op->d.sbsref_subscript.state;
+	JsonbSubWorkspace *workspace = (JsonbSubWorkspace *) sbsrefstate->workspace;
+
+	/*
+	 * In case if the first subscript is an integer, the source jsonb is
+	 * expected to be an array. This information is not used directly, all such
+	 * cases are handled within corresponding jsonb assign functions. But if
+	 * the source jsonb is NULL the expected type will be used to construct an
+	 * empty source.
+	 */
+	if (sbsrefstate->numupper > 0 && sbsrefstate->upperprovided[0] &&
+		!sbsrefstate->upperindexnull[0] && workspace->indexOid[0] == INT4OID)
+		workspace->expectArray = true;
+
+	/* Process upper subscripts */
+	for (int i = 0; i < sbsrefstate->numupper; i++)
+	{
+		if (sbsrefstate->upperprovided[i])
+		{
+			/* If any index expr yields NULL, result is NULL or error */
+			if (sbsrefstate->upperindexnull[i])
+			{
+				if (sbsrefstate->isassignment)
+					ereport(ERROR,
+							(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+							 errmsg("jsonb subscript in assignment must not be null")));
+				*op->resnull = true;
+				return false;
+			}
+
+			/*
+			 * For jsonb fetch and assign functions we need to provide path in
+			 * text format. Convert if it's not already text.
+			 */
+			if (workspace->indexOid[i] == INT4OID)
+			{
+				Datum	datum = sbsrefstate->upperindex[i];
+				char   *cs = DatumGetCString(DirectFunctionCall1(int4out, datum));
+				workspace->index[i] = CStringGetTextDatum(cs);
+			}
+			else
+				workspace->index[i] = sbsrefstate->upperindex[i];
+		}
+	}
+
+	return true;
+}
+
+/*
+ * Evaluate SubscriptingRef fetch for a jsonb element.
+ *
+ * Source container is in step's result variable (it's known not NULL, since
+ * we set fetch_strict to true).
+ */
+static void
+jsonb_subscript_fetch(ExprState *state,
+					  ExprEvalStep *op,
+					  ExprContext *econtext)
+{
+	SubscriptingRefState *sbsrefstate = op->d.sbsref.state;
+	JsonbSubWorkspace *workspace = (JsonbSubWorkspace *) sbsrefstate->workspace;
+	Jsonb		*jsonbSource;
+
+	/* Should not get here if source jsonb (or any subscript) is null */
+	Assert(!(*op->resnull));
+
+	jsonbSource = DatumGetJsonbP(*op->resvalue);
+	*op->resvalue = jsonb_get_element(jsonbSource,
+									  workspace->index,
+									  sbsrefstate->numupper,
+									  op->resnull,
+									  false);
+}
+
+/*
+ * Evaluate SubscriptingRef assignment for a jsonb element assignment.
+ *
+ * Input container (possibly null) is in result area, replacement value is in
+ * SubscriptingRefState's replacevalue/replacenull.
+ */
+static void
+jsonb_subscript_assign(ExprState *state,
+					   ExprEvalStep *op,
+					   ExprContext *econtext)
+{
+	SubscriptingRefState *sbsrefstate = op->d.sbsref.state;
+	JsonbSubWorkspace *workspace = (JsonbSubWorkspace *) sbsrefstate->workspace;
+	Jsonb		*jsonbSource;
+	JsonbValue	*replacevalue;
+
+	if (sbsrefstate->replacenull)
+	{
+		replacevalue = (JsonbValue *) palloc(sizeof(JsonbValue));
+		replacevalue->type = jbvNull;
+	}
+	else
+		replacevalue =
+			JsonbToJsonbValue(DatumGetJsonbP(sbsrefstate->replacevalue));
+
+	/*
+	 * In case if the input container is null, set up an empty jsonb and
+	 * proceed with the assignment.
+	 */
+	if (*op->resnull)
+	{
+		JsonbValue *newSource = (JsonbValue *) palloc(sizeof(JsonbValue));
+
+		/*
+		 * To avoid any surprising results, set up an empty jsonb array in case
+		 * of an array is expected (i.e. the first subscript is integer),
+		 * otherwise jsonb object.
+		 */
+		if (workspace->expectArray)
+		{
+			newSource->type = jbvArray;
+			newSource->val.array.nElems = 0;
+			newSource->val.array.rawScalar = false;
+		}
+		else
+		{
+			newSource->type = jbvObject;
+			newSource->val.object.nPairs = 0;
+		}
+
+		jsonbSource = JsonbValueToJsonb(newSource);
+		*op->resnull = false;
+	}
+	else
+		jsonbSource = DatumGetJsonbP(*op->resvalue);
+
+	*op->resvalue = jsonb_set_element(jsonbSource,
+									  workspace->index,
+									  sbsrefstate->numupper,
+									  replacevalue);
+	/* The result is never NULL, so no need to change *op->resnull */
+}
+
+/*
+ * Compute old jsonb element value for a SubscriptingRef assignment
+ * expression.  Will only be called if the new-value subexpression
+ * contains SubscriptingRef or FieldStore.  This is the same as the
+ * regular fetch case, except that we have to handle a null jsonb,
+ * and the value should be stored into the SubscriptingRefState's
+ * prevvalue/prevnull fields.
+ */
+static void
+jsonb_subscript_fetch_old(ExprState *state,
+						  ExprEvalStep *op,
+						  ExprContext *econtext)
+{
+	SubscriptingRefState *sbsrefstate = op->d.sbsref.state;
+
+	if (*op->resnull)
+	{
+		/* whole jsonb is null, so any element is too */
+		sbsrefstate->prevvalue = (Datum) 0;
+		sbsrefstate->prevnull = true;
+	}
+	else
+	{
+		Jsonb	*jsonbSource = DatumGetJsonbP(*op->resvalue);
+		sbsrefstate->prevvalue = jsonb_get_element(jsonbSource,
+									  			   sbsrefstate->upperindex,
+									  			   sbsrefstate->numupper,
+												   &sbsrefstate->prevnull,
+												   false);
+	}
+}
+
+/*
+ * Set up execution state for a jsonb subscript operation. Opposite to the
+ * arrays subscription, there is no limit for number of subscripts as jsonb
+ * type itself doesn't have nesting limits.
+ */
+static void
+jsonb_exec_setup(const SubscriptingRef *sbsref,
+				 SubscriptingRefState *sbsrefstate,
+				 SubscriptExecSteps *methods)
+{
+	JsonbSubWorkspace *workspace;
+	ListCell   *lc;
+	int			nupper = sbsref->refupperindexpr->length;
+	char	   *ptr;
+
+	/* Allocate type-specific workspace with space for per-subscript data */
+	workspace = palloc0(MAXALIGN(sizeof(JsonbSubWorkspace)) +
+					    nupper * (sizeof(Datum) + sizeof(Oid)));
+	workspace->expectArray = false;
+	ptr = ((char *) workspace) + MAXALIGN(sizeof(JsonbSubWorkspace));
+	workspace->indexOid = (Oid *) ptr;
+	ptr += nupper * sizeof(Oid);
+	workspace->index = (Datum *) ptr;
+
+	sbsrefstate->workspace = workspace;
+
+	/* Collect subscript data types necessary at execution time */
+	foreach(lc, sbsref->refupperindexpr)
+	{
+		Node   *expr = lfirst(lc);
+		int 	i = foreach_current_index(lc);
+
+		workspace->indexOid[i] = exprType(expr);
+	}
+
+	/*
+	 * Pass back pointers to appropriate step execution functions.
+	 */
+	methods->sbs_check_subscripts = jsonb_subscript_check_subscripts;
+	methods->sbs_fetch = jsonb_subscript_fetch;
+	methods->sbs_assign = jsonb_subscript_assign;
+	methods->sbs_fetch_old = jsonb_subscript_fetch_old;
+}
+
+/*
+ * jsonb_subscript_handler
+ *		Subscripting handler for jsonb.
+ *
+ */
+Datum
+jsonb_subscript_handler(PG_FUNCTION_ARGS)
+{
+	static const SubscriptRoutines sbsroutines = {
+		.transform = jsonb_subscript_transform,
+		.exec_setup = jsonb_exec_setup,
+		.fetch_strict = true,		/* fetch returns NULL for NULL inputs */
+		.fetch_leakproof = true,	/* fetch returns NULL for bad subscript */
+		.store_leakproof = false	/* ... but assignment throws error */
+	};
+
+	PG_RETURN_POINTER(&sbsroutines);
+}
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 69100feab7..5a0ba6b220 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -461,18 +461,18 @@ static Datum populate_domain(DomainIOData *io, Oid typid, const char *colname,
 /* functions supporting jsonb_delete, jsonb_set and jsonb_concat */
 static JsonbValue *IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
 								  JsonbParseState **state);
-static JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
+extern JsonbValue *setPath(JsonbIterator **it, Datum *path_elems,
 						   bool *path_nulls, int path_len,
-						   JsonbParseState **st, int level, Jsonb *newval,
+						   JsonbParseState **st, int level, JsonbValue *newval,
 						   int op_type);
 static void setPathObject(JsonbIterator **it, Datum *path_elems,
 						  bool *path_nulls, int path_len, JsonbParseState **st,
 						  int level,
-						  Jsonb *newval, uint32 npairs, int op_type);
+						  JsonbValue *newval, uint32 npairs, int op_type);
 static void setPathArray(JsonbIterator **it, Datum *path_elems,
 						 bool *path_nulls, int path_len, JsonbParseState **st,
-						 int level, Jsonb *newval, uint32 nelems, int op_type);
-static void addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb);
+						 int level,
+						 JsonbValue *newval, uint32 nelems, int op_type);
 
 /* function supporting iterate_json_values */
 static void iterate_values_scalar(void *state, char *token, JsonTokenType tokentype);
@@ -1448,13 +1448,9 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 	ArrayType  *path = PG_GETARG_ARRAYTYPE_P(1);
 	Datum	   *pathtext;
 	bool	   *pathnulls;
+	bool		isnull;
 	int			npath;
-	int			i;
-	bool		have_object = false,
-				have_array = false;
-	JsonbValue *jbvp = NULL;
-	JsonbValue	jbvbuf;
-	JsonbContainer *container;
+	Datum		res;
 
 	/*
 	 * If the array contains any null elements, return NULL, on the grounds
@@ -1469,9 +1465,26 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 	deconstruct_array(path, TEXTOID, -1, false, TYPALIGN_INT,
 					  &pathtext, &pathnulls, &npath);
 
-	/* Identify whether we have object, array, or scalar at top-level */
-	container = &jb->root;
+	res = jsonb_get_element(jb, pathtext, npath, &isnull, as_text);
 
+	if (isnull)
+		PG_RETURN_NULL();
+	else
+		PG_RETURN_DATUM(res);
+}
+
+Datum
+jsonb_get_element(Jsonb *jb, Datum *path, int npath, bool *isnull, bool as_text)
+{
+	JsonbContainer *container = &jb->root;
+	JsonbValue	   *jbvp = NULL;
+	int				i;
+	bool			have_object = false,
+					have_array = false;
+
+	*isnull = false;
+
+	/* Identify whether we have object, array, or scalar at top-level */
 	if (JB_ROOT_IS_OBJECT(jb))
 		have_object = true;
 	else if (JB_ROOT_IS_ARRAY(jb) && !JB_ROOT_IS_SCALAR(jb))
@@ -1496,7 +1509,7 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 	{
 		if (as_text)
 		{
-			PG_RETURN_TEXT_P(cstring_to_text(JsonbToCString(NULL,
+			return PointerGetDatum(cstring_to_text(JsonbToCString(NULL,
 															container,
 															VARSIZE(jb))));
 		}
@@ -1512,22 +1525,25 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 		if (have_object)
 		{
 			jbvp = getKeyJsonValueFromContainer(container,
-												VARDATA(pathtext[i]),
-												VARSIZE(pathtext[i]) - VARHDRSZ,
-												&jbvbuf);
+												VARDATA(path[i]),
+												VARSIZE(path[i]) - VARHDRSZ,
+												NULL);
 		}
 		else if (have_array)
 		{
 			long		lindex;
 			uint32		index;
-			char	   *indextext = TextDatumGetCString(pathtext[i]);
+			char	   *indextext = TextDatumGetCString(path[i]);
 			char	   *endptr;
 
 			errno = 0;
 			lindex = strtol(indextext, &endptr, 10);
 			if (endptr == indextext || *endptr != '\0' || errno != 0 ||
 				lindex > INT_MAX || lindex < INT_MIN)
-				PG_RETURN_NULL();
+			{
+				*isnull = true;
+				return PointerGetDatum(NULL);
+			}
 
 			if (lindex >= 0)
 			{
@@ -1545,7 +1561,10 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 				nelements = JsonContainerSize(container);
 
 				if (-lindex > nelements)
-					PG_RETURN_NULL();
+				{
+					*isnull = true;
+					return PointerGetDatum(NULL);
+				}
 				else
 					index = nelements + lindex;
 			}
@@ -1555,11 +1574,15 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 		else
 		{
 			/* scalar, extraction yields a null */
-			PG_RETURN_NULL();
+			*isnull = true;
+			return PointerGetDatum(NULL);
 		}
 
 		if (jbvp == NULL)
-			PG_RETURN_NULL();
+		{
+			*isnull = true;
+			return PointerGetDatum(NULL);
+		}
 		else if (i == npath - 1)
 			break;
 
@@ -1581,9 +1604,12 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 	if (as_text)
 	{
 		if (jbvp->type == jbvNull)
-			PG_RETURN_NULL();
+		{
+			*isnull = true;
+			return PointerGetDatum(NULL);
+		}
 
-		PG_RETURN_TEXT_P(JsonbValueAsText(jbvp));
+		return PointerGetDatum(JsonbValueAsText(jbvp));
 	}
 	else
 	{
@@ -1594,6 +1620,28 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 	}
 }
 
+Datum
+jsonb_set_element(Jsonb* jb, Datum *path, int path_len,
+				  JsonbValue *newval)
+{
+	JsonbValue		   *res;
+	JsonbParseState    *state = NULL;
+	JsonbIterator 	   *it;
+	bool			   *path_nulls = palloc0(path_len * sizeof(bool));
+
+	if (newval->type == jbvArray && newval->val.array.rawScalar)
+		*newval = newval->val.array.elems[0];
+
+	it = JsonbIteratorInit(&jb->root);
+
+	res = setPath(&it, path, path_nulls, path_len, &state, 0,
+				  newval, JB_PATH_CREATE);
+
+	pfree(path_nulls);
+
+	PG_RETURN_JSONB_P(JsonbValueToJsonb(res));
+}
+
 /*
  * Return the text representation of the given JsonbValue.
  */
@@ -4151,58 +4199,6 @@ jsonb_strip_nulls(PG_FUNCTION_ARGS)
 	PG_RETURN_POINTER(JsonbValueToJsonb(res));
 }
 
-/*
- * Add values from the jsonb to the parse state.
- *
- * If the parse state container is an object, the jsonb is pushed as
- * a value, not a key.
- *
- * This needs to be done using an iterator because pushJsonbValue doesn't
- * like getting jbvBinary values, so we can't just push jb as a whole.
- */
-static void
-addJsonbToParseState(JsonbParseState **jbps, Jsonb *jb)
-{
-	JsonbIterator *it;
-	JsonbValue *o = &(*jbps)->contVal;
-	JsonbValue	v;
-	JsonbIteratorToken type;
-
-	it = JsonbIteratorInit(&jb->root);
-
-	Assert(o->type == jbvArray || o->type == jbvObject);
-
-	if (JB_ROOT_IS_SCALAR(jb))
-	{
-		(void) JsonbIteratorNext(&it, &v, false);	/* skip array header */
-		Assert(v.type == jbvArray);
-		(void) JsonbIteratorNext(&it, &v, false);	/* fetch scalar value */
-
-		switch (o->type)
-		{
-			case jbvArray:
-				(void) pushJsonbValue(jbps, WJB_ELEM, &v);
-				break;
-			case jbvObject:
-				(void) pushJsonbValue(jbps, WJB_VALUE, &v);
-				break;
-			default:
-				elog(ERROR, "unexpected parent of nested structure");
-		}
-	}
-	else
-	{
-		while ((type = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
-		{
-			if (type == WJB_KEY || type == WJB_VALUE || type == WJB_ELEM)
-				(void) pushJsonbValue(jbps, type, &v);
-			else
-				(void) pushJsonbValue(jbps, type, NULL);
-		}
-	}
-
-}
-
 /*
  * SQL function jsonb_pretty (jsonb)
  *
@@ -4474,7 +4470,8 @@ jsonb_set(PG_FUNCTION_ARGS)
 {
 	Jsonb	   *in = PG_GETARG_JSONB_P(0);
 	ArrayType  *path = PG_GETARG_ARRAYTYPE_P(1);
-	Jsonb	   *newval = PG_GETARG_JSONB_P(2);
+	Jsonb	   *newjsonb = PG_GETARG_JSONB_P(2);
+	JsonbValue *newval = JsonbToJsonbValue(newjsonb);
 	bool		create = PG_GETARG_BOOL(3);
 	JsonbValue *res = NULL;
 	Datum	   *path_elems;
@@ -4632,7 +4629,8 @@ jsonb_insert(PG_FUNCTION_ARGS)
 {
 	Jsonb	   *in = PG_GETARG_JSONB_P(0);
 	ArrayType  *path = PG_GETARG_ARRAYTYPE_P(1);
-	Jsonb	   *newval = PG_GETARG_JSONB_P(2);
+	Jsonb	   *newjsonb = PG_GETARG_JSONB_P(2);
+	JsonbValue *newval = JsonbToJsonbValue(newjsonb);
 	bool		after = PG_GETARG_BOOL(3);
 	JsonbValue *res = NULL;
 	Datum	   *path_elems;
@@ -4787,10 +4785,10 @@ IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
  * All path elements before the last must already exist
  * whatever bits in op_type are set, or nothing is done.
  */
-static JsonbValue *
+JsonbValue *
 setPath(JsonbIterator **it, Datum *path_elems,
 		bool *path_nulls, int path_len,
-		JsonbParseState **st, int level, Jsonb *newval, int op_type)
+		JsonbParseState **st, int level, JsonbValue *newval, int op_type)
 {
 	JsonbValue	v;
 	JsonbIteratorToken r;
@@ -4843,11 +4841,11 @@ setPath(JsonbIterator **it, Datum *path_elems,
 static void
 setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			  int path_len, JsonbParseState **st, int level,
-			  Jsonb *newval, uint32 npairs, int op_type)
+			  JsonbValue *newval, uint32 npairs, int op_type)
 {
-	JsonbValue	v;
 	int			i;
-	JsonbValue	k;
+	JsonbValue	k,
+				v;
 	bool		done = false;
 
 	if (level >= path_len || path_nulls[level])
@@ -4864,7 +4862,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 		newkey.val.string.val = VARDATA_ANY(path_elems[level]);
 
 		(void) pushJsonbValue(st, WJB_KEY, &newkey);
-		addJsonbToParseState(st, newval);
+		(void) pushJsonbValue(st, WJB_VALUE, newval);
 	}
 
 	for (i = 0; i < npairs; i++)
@@ -4895,7 +4893,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 				if (!(op_type & JB_PATH_DELETE))
 				{
 					(void) pushJsonbValue(st, WJB_KEY, &k);
-					addJsonbToParseState(st, newval);
+					(void) pushJsonbValue(st, WJB_VALUE, newval);
 				}
 				done = true;
 			}
@@ -4918,7 +4916,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 				newkey.val.string.val = VARDATA_ANY(path_elems[level]);
 
 				(void) pushJsonbValue(st, WJB_KEY, &newkey);
-				addJsonbToParseState(st, newval);
+				(void) pushJsonbValue(st, WJB_VALUE, newval);
 			}
 
 			(void) pushJsonbValue(st, r, &k);
@@ -4950,7 +4948,7 @@ setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 static void
 setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			 int path_len, JsonbParseState **st, int level,
-			 Jsonb *newval, uint32 nelems, int op_type)
+			 JsonbValue *newval, uint32 nelems, int op_type)
 {
 	JsonbValue	v;
 	int			idx,
@@ -4998,7 +4996,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 		(op_type & JB_PATH_CREATE_OR_INSERT))
 	{
 		Assert(newval != NULL);
-		addJsonbToParseState(st, newval);
+		(void) pushJsonbValue(st, WJB_ELEM, newval);
 		done = true;
 	}
 
@@ -5014,7 +5012,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 				r = JsonbIteratorNext(it, &v, true);	/* skip */
 
 				if (op_type & (JB_PATH_INSERT_BEFORE | JB_PATH_CREATE))
-					addJsonbToParseState(st, newval);
+					(void) pushJsonbValue(st, WJB_ELEM, newval);
 
 				/*
 				 * We should keep current value only in case of
@@ -5025,7 +5023,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 					(void) pushJsonbValue(st, r, &v);
 
 				if (op_type & (JB_PATH_INSERT_AFTER | JB_PATH_REPLACE))
-					addJsonbToParseState(st, newval);
+					(void) pushJsonbValue(st, WJB_ELEM, newval);
 
 				done = true;
 			}
@@ -5059,7 +5057,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			if ((op_type & JB_PATH_CREATE_OR_INSERT) && !done &&
 				level == path_len - 1 && i == nelems - 1)
 			{
-				addJsonbToParseState(st, newval);
+				(void) pushJsonbValue(st, WJB_ELEM, newval);
 			}
 		}
 	}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 139f4a08bd..feae8cc4b0 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -11239,6 +11239,10 @@
 { oid => '9256', descr => 'raw array subscripting support',
   proname => 'raw_array_subscript_handler', prorettype => 'internal',
   proargtypes => 'internal', prosrc => 'raw_array_subscript_handler' },
+# type subscripting support
+{ oid => '6098', descr => 'jsonb subscripting logic',
+  proname => 'jsonb_subscript_handler', prorettype => 'internal',
+  proargtypes => 'internal', prosrc => 'jsonb_subscript_handler' },
 
 # collation management functions
 { oid => '3445', descr => 'import collations from operating system',
diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat
index 62018f063a..4a530ca907 100644
--- a/src/include/catalog/pg_type.dat
+++ b/src/include/catalog/pg_type.dat
@@ -444,7 +444,8 @@
 { oid => '3802', array_type_oid => '3807', descr => 'Binary JSON',
   typname => 'jsonb', typlen => '-1', typbyval => 'f', typcategory => 'U',
   typinput => 'jsonb_in', typoutput => 'jsonb_out', typreceive => 'jsonb_recv',
-  typsend => 'jsonb_send', typalign => 'i', typstorage => 'x' },
+  typsend => 'jsonb_send', typalign => 'i', typstorage => 'x',
+  typsubscript => 'jsonb_subscript_handler' },
 { oid => '4072', array_type_oid => '4073', descr => 'JSON path',
   typname => 'jsonpath', typlen => '-1', typbyval => 'f', typcategory => 'U',
   typinput => 'jsonpath_in', typoutput => 'jsonpath_out',
diff --git a/src/include/utils/jsonb.h b/src/include/utils/jsonb.h
index 5860011693..64f1ccbf77 100644
--- a/src/include/utils/jsonb.h
+++ b/src/include/utils/jsonb.h
@@ -392,6 +392,7 @@ extern JsonbValue *pushJsonbValue(JsonbParseState **pstate,
 extern JsonbIterator *JsonbIteratorInit(JsonbContainer *container);
 extern JsonbIteratorToken JsonbIteratorNext(JsonbIterator **it, JsonbValue *val,
 											bool skipNested);
+extern JsonbValue *JsonbToJsonbValue(Jsonb *jsonb);
 extern Jsonb *JsonbValueToJsonb(JsonbValue *val);
 extern bool JsonbDeepContains(JsonbIterator **val,
 							  JsonbIterator **mContained);
@@ -407,5 +408,8 @@ extern char *JsonbToCStringIndent(StringInfo out, JsonbContainer *in,
 extern bool JsonbExtractScalar(JsonbContainer *jbc, JsonbValue *res);
 extern const char *JsonbTypeName(JsonbValue *jb);
 
-
+extern Datum jsonb_set_element(Jsonb *jb, Datum *path, int path_len,
+							   JsonbValue *newval);
+extern Datum jsonb_get_element(Jsonb *jb, Datum *path, int npath,
+							   bool *isnull, bool as_text);
 #endif							/* __JSONB_H__ */
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 1e6c6ef200..bb3f25ec3f 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -4599,7 +4599,7 @@ select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'use_
  {"a": 1, "b": null}
 (1 row)
 
-\pset null
+\pset null ''
 -- jsonb_insert
 select jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"');
          jsonb_insert          
@@ -4729,6 +4729,276 @@ HINT:  Try using the function jsonb_set to replace key value.
 select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"', true);
 ERROR:  cannot replace existing key
 HINT:  Try using the function jsonb_set to replace key value.
+-- jsonb subscript
+select ('123'::jsonb)['a'];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('123'::jsonb)[0];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('123'::jsonb)[NULL];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('{"a": 1}'::jsonb)['a'];
+ jsonb 
+-------
+ 1
+(1 row)
+
+select ('{"a": 1}'::jsonb)[0];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('{"a": 1}'::jsonb)['not_exist'];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('{"a": 1}'::jsonb)[NULL];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('[1, "2", null]'::jsonb)['a'];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[0];
+ jsonb 
+-------
+ 1
+(1 row)
+
+select ('[1, "2", null]'::jsonb)['1'];
+ jsonb 
+-------
+ "2"
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[1.0];
+ERROR:  subscript type is not supported
+LINE 1: select ('[1, "2", null]'::jsonb)[1.0];
+                                         ^
+HINT:  Jsonb subscript must be coercet to either integer or text
+select ('[1, "2", null]'::jsonb)[2];
+ jsonb 
+-------
+ null
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[3];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[-2];
+ jsonb 
+-------
+ "2"
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[1]['a'];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('[1, "2", null]'::jsonb)[1][0];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['b'];
+ jsonb 
+-------
+ "c"
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'];
+   jsonb   
+-----------
+ [1, 2, 3]
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'][1];
+ jsonb 
+-------
+ 2
+(1 row)
+
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d']['a'];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1'];
+     jsonb     
+---------------
+ {"a2": "aaa"}
+(1 row)
+
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2'];
+ jsonb 
+-------
+ "aaa"
+(1 row)
+
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2']['a3'];
+ jsonb 
+-------
+ 
+(1 row)
+
+select ('{"a": ["a1", {"b1": ["aaa", "bbb", "ccc"]}], "b": "bb"}'::jsonb)['a'][1]['b1'];
+         jsonb         
+-----------------------
+ ["aaa", "bbb", "ccc"]
+(1 row)
+
+select ('{"a": ["a1", {"b1": ["aaa", "bbb", "ccc"]}], "b": "bb"}'::jsonb)['a'][1]['b1'][2];
+ jsonb 
+-------
+ "ccc"
+(1 row)
+
+-- slices are not supported
+select ('{"a": 1}'::jsonb)['a':'b'];
+ERROR:  jsonb subscript does not support slices
+LINE 1: select ('{"a": 1}'::jsonb)['a':'b'];
+                                       ^
+select ('[1, "2", null]'::jsonb)[1:2];
+ERROR:  jsonb subscript does not support slices
+LINE 1: select ('[1, "2", null]'::jsonb)[1:2];
+                                           ^
+select ('[1, "2", null]'::jsonb)[:2];
+ERROR:  jsonb subscript does not support slices
+LINE 1: select ('[1, "2", null]'::jsonb)[:2];
+                                          ^
+select ('[1, "2", null]'::jsonb)[1:];
+ERROR:  jsonb subscript does not support slices
+LINE 1: select ('[1, "2", null]'::jsonb)[1:];
+                                         ^
+select ('[1, "2", null]'::jsonb)[:];
+ERROR:  jsonb subscript does not support slices
+create TEMP TABLE test_jsonb_subscript (
+       id int,
+       test_json jsonb
+);
+insert into test_jsonb_subscript values
+(1, '{}'), -- empty jsonb
+(2, '{"key": "value"}'); -- jsonb with data
+-- update empty jsonb
+update test_jsonb_subscript set test_json['a'] = '1' where id = 1;
+select * from test_jsonb_subscript;
+ id |    test_json     
+----+------------------
+  2 | {"key": "value"}
+  1 | {"a": 1}
+(2 rows)
+
+-- update jsonb with some data
+update test_jsonb_subscript set test_json['a'] = '1' where id = 2;
+select * from test_jsonb_subscript;
+ id |        test_json         
+----+--------------------------
+  1 | {"a": 1}
+  2 | {"a": 1, "key": "value"}
+(2 rows)
+
+-- replace jsonb
+update test_jsonb_subscript set test_json['a'] = '"test"';
+select * from test_jsonb_subscript;
+ id |           test_json           
+----+-------------------------------
+  1 | {"a": "test"}
+  2 | {"a": "test", "key": "value"}
+(2 rows)
+
+-- replace by object
+update test_jsonb_subscript set test_json['a'] = '{"b": 1}'::jsonb;
+select * from test_jsonb_subscript;
+ id |            test_json            
+----+---------------------------------
+  1 | {"a": {"b": 1}}
+  2 | {"a": {"b": 1}, "key": "value"}
+(2 rows)
+
+-- replace by array
+update test_jsonb_subscript set test_json['a'] = '[1, 2, 3]'::jsonb;
+select * from test_jsonb_subscript;
+ id |            test_json             
+----+----------------------------------
+  1 | {"a": [1, 2, 3]}
+  2 | {"a": [1, 2, 3], "key": "value"}
+(2 rows)
+
+-- use jsonb subscription in where clause
+select * from test_jsonb_subscript where test_json['key'] = '"value"';
+ id |            test_json             
+----+----------------------------------
+  2 | {"a": [1, 2, 3], "key": "value"}
+(1 row)
+
+select * from test_jsonb_subscript where test_json['key_doesnt_exists'] = '"value"';
+ id | test_json 
+----+-----------
+(0 rows)
+
+select * from test_jsonb_subscript where test_json['key'] = '"wrong_value"';
+ id | test_json 
+----+-----------
+(0 rows)
+
+-- NULL
+update test_jsonb_subscript set test_json[NULL] = '1';
+ERROR:  jsonb subscript in assignment must not be null
+update test_jsonb_subscript set test_json['another_key'] = NULL;
+select * from test_jsonb_subscript;
+ id |                       test_json                       
+----+-------------------------------------------------------
+  1 | {"a": [1, 2, 3], "another_key": null}
+  2 | {"a": [1, 2, 3], "key": "value", "another_key": null}
+(2 rows)
+
+-- NULL as jsonb source
+insert into test_jsonb_subscript values (3, NULL);
+update test_jsonb_subscript set test_json['a'] = '1' where id = 3;
+select * from test_jsonb_subscript;
+ id |                       test_json                       
+----+-------------------------------------------------------
+  1 | {"a": [1, 2, 3], "another_key": null}
+  2 | {"a": [1, 2, 3], "key": "value", "another_key": null}
+  3 | {"a": 1}
+(3 rows)
+
+update test_jsonb_subscript set test_json = NULL where id = 3;
+update test_jsonb_subscript set test_json[0] = '1';
+select * from test_jsonb_subscript;
+ id |                           test_json                           
+----+---------------------------------------------------------------
+  1 | {"0": 1, "a": [1, 2, 3], "another_key": null}
+  2 | {"0": 1, "a": [1, 2, 3], "key": "value", "another_key": null}
+  3 | [1]
+(3 rows)
+
 -- jsonb to tsvector
 select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb);
                                 to_tsvector                                
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index b6409767f6..20aa8fe0e2 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -1177,7 +1177,7 @@ select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'retu
 select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'delete_key') as delete_key;
 select jsonb_set_lax('{"a":1,"b":2}', '{b}', null, null_value_treatment => 'use_json_null') as use_json_null;
 
-\pset null
+\pset null ''
 
 -- jsonb_insert
 select jsonb_insert('{"a": [0,1,2]}', '{a, 1}', '"new_value"');
@@ -1208,6 +1208,88 @@ select jsonb_insert('{"a": {"b": "value"}}', '{a, c}', '"new_value"', true);
 select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"');
 select jsonb_insert('{"a": {"b": "value"}}', '{a, b}', '"new_value"', true);
 
+-- jsonb subscript
+select ('123'::jsonb)['a'];
+select ('123'::jsonb)[0];
+select ('123'::jsonb)[NULL];
+select ('{"a": 1}'::jsonb)['a'];
+select ('{"a": 1}'::jsonb)[0];
+select ('{"a": 1}'::jsonb)['not_exist'];
+select ('{"a": 1}'::jsonb)[NULL];
+select ('[1, "2", null]'::jsonb)['a'];
+select ('[1, "2", null]'::jsonb)[0];
+select ('[1, "2", null]'::jsonb)['1'];
+select ('[1, "2", null]'::jsonb)[1.0];
+select ('[1, "2", null]'::jsonb)[2];
+select ('[1, "2", null]'::jsonb)[3];
+select ('[1, "2", null]'::jsonb)[-2];
+select ('[1, "2", null]'::jsonb)[1]['a'];
+select ('[1, "2", null]'::jsonb)[1][0];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['b'];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d'][1];
+select ('{"a": 1, "b": "c", "d": [1, 2, 3]}'::jsonb)['d']['a'];
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1'];
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2'];
+select ('{"a": {"a1": {"a2": "aaa"}}, "b": "bbb", "c": "ccc"}'::jsonb)['a']['a1']['a2']['a3'];
+select ('{"a": ["a1", {"b1": ["aaa", "bbb", "ccc"]}], "b": "bb"}'::jsonb)['a'][1]['b1'];
+select ('{"a": ["a1", {"b1": ["aaa", "bbb", "ccc"]}], "b": "bb"}'::jsonb)['a'][1]['b1'][2];
+
+-- slices are not supported
+select ('{"a": 1}'::jsonb)['a':'b'];
+select ('[1, "2", null]'::jsonb)[1:2];
+select ('[1, "2", null]'::jsonb)[:2];
+select ('[1, "2", null]'::jsonb)[1:];
+select ('[1, "2", null]'::jsonb)[:];
+
+create TEMP TABLE test_jsonb_subscript (
+       id int,
+       test_json jsonb
+);
+
+insert into test_jsonb_subscript values
+(1, '{}'), -- empty jsonb
+(2, '{"key": "value"}'); -- jsonb with data
+
+-- update empty jsonb
+update test_jsonb_subscript set test_json['a'] = '1' where id = 1;
+select * from test_jsonb_subscript;
+
+-- update jsonb with some data
+update test_jsonb_subscript set test_json['a'] = '1' where id = 2;
+select * from test_jsonb_subscript;
+
+-- replace jsonb
+update test_jsonb_subscript set test_json['a'] = '"test"';
+select * from test_jsonb_subscript;
+
+-- replace by object
+update test_jsonb_subscript set test_json['a'] = '{"b": 1}'::jsonb;
+select * from test_jsonb_subscript;
+
+-- replace by array
+update test_jsonb_subscript set test_json['a'] = '[1, 2, 3]'::jsonb;
+select * from test_jsonb_subscript;
+
+-- use jsonb subscription in where clause
+select * from test_jsonb_subscript where test_json['key'] = '"value"';
+select * from test_jsonb_subscript where test_json['key_doesnt_exists'] = '"value"';
+select * from test_jsonb_subscript where test_json['key'] = '"wrong_value"';
+
+-- NULL
+update test_jsonb_subscript set test_json[NULL] = '1';
+update test_jsonb_subscript set test_json['another_key'] = NULL;
+select * from test_jsonb_subscript;
+
+-- NULL as jsonb source
+insert into test_jsonb_subscript values (3, NULL);
+update test_jsonb_subscript set test_json['a'] = '1' where id = 3;
+select * from test_jsonb_subscript;
+
+update test_jsonb_subscript set test_json = NULL where id = 3;
+update test_jsonb_subscript set test_json[0] = '1';
+select * from test_jsonb_subscript;
+
 -- jsonb to tsvector
 select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb);
 
-- 
2.21.0


--bnxkghd4qzx6dtmd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v41-0002-Filling-gaps-in-jsonb.patch"



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

* Re: Convert macros to static inline functions
@ 2024-12-27 10:16 Peter Eisentraut <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Peter Eisentraut @ 2024-12-27 10:16 UTC (permalink / raw)
  To: pgsql-hackers

On 16.05.22 10:27, Peter Eisentraut wrote:
> Inspired by [0], I looked to convert more macros to inline functions. 

This is an older thread where I left something unfinished:

> Note 2: Many macros in htup_details.h operate both on HeapTupleHeader 
> and on MinimalTuple, so converting them to a function doesn't work in a 
> straightforward way.  I have some in-progress work in that area, but I 
> have not included any of that here.

Here is the patch set for this.

There are actually only two macros that operate on both HeapTupleHeader 
and MinimalTuple, so it wasn't as much as I had written above.  I just 
left those as macros.  I converted the rest to inline functions in a 
straightforward way as before.  A small amount of reordering was necessary.

But just for language-nerd fun, I'm including here an additional patch 
showing how the remaining ones could be done with C11 generic selection. 
  I'm not planning to commit that one at this time.

From 422181163bfe1a292ee51941a007d54bc127ccec Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 27 Dec 2024 11:07:35 +0100
Subject: [PATCH 1/3] Add some const decorations (htup.h)

---
 src/backend/access/heap/heapam.c  | 6 +++---
 src/backend/utils/time/combocid.c | 6 +++---
 src/include/access/htup.h         | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 329e727f80d..75171e8f2c0 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -7419,10 +7419,10 @@ MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask)
  * checking the hint bits.
  */
 TransactionId
-HeapTupleGetUpdateXid(HeapTupleHeader tuple)
+HeapTupleGetUpdateXid(const HeapTupleHeaderData *tup)
 {
-	return MultiXactIdGetUpdateXid(HeapTupleHeaderGetRawXmax(tuple),
-								   tuple->t_infomask);
+	return MultiXactIdGetUpdateXid(HeapTupleHeaderGetRawXmax(tup),
+								   tup->t_infomask);
 }
 
 /*
diff --git a/src/backend/utils/time/combocid.c b/src/backend/utils/time/combocid.c
index f85510b74ff..1ecdc93b7e2 100644
--- a/src/backend/utils/time/combocid.c
+++ b/src/backend/utils/time/combocid.c
@@ -101,7 +101,7 @@ static CommandId GetRealCmax(CommandId combocid);
  */
 
 CommandId
-HeapTupleHeaderGetCmin(HeapTupleHeader tup)
+HeapTupleHeaderGetCmin(const HeapTupleHeaderData *tup)
 {
 	CommandId	cid = HeapTupleHeaderGetRawCommandId(tup);
 
@@ -115,7 +115,7 @@ HeapTupleHeaderGetCmin(HeapTupleHeader tup)
 }
 
 CommandId
-HeapTupleHeaderGetCmax(HeapTupleHeader tup)
+HeapTupleHeaderGetCmax(const HeapTupleHeaderData *tup)
 {
 	CommandId	cid = HeapTupleHeaderGetRawCommandId(tup);
 
@@ -150,7 +150,7 @@ HeapTupleHeaderGetCmax(HeapTupleHeader tup)
  * changes the tuple in shared buffers.
  */
 void
-HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
+HeapTupleHeaderAdjustCmax(const HeapTupleHeaderData *tup,
 						  CommandId *cmax,
 						  bool *iscombo)
 {
diff --git a/src/include/access/htup.h b/src/include/access/htup.h
index 116cb1bb273..a5f1ef2441d 100644
--- a/src/include/access/htup.h
+++ b/src/include/access/htup.h
@@ -78,12 +78,12 @@ typedef HeapTupleData *HeapTuple;
 #define HeapTupleIsValid(tuple) PointerIsValid(tuple)
 
 /* HeapTupleHeader functions implemented in utils/time/combocid.c */
-extern CommandId HeapTupleHeaderGetCmin(HeapTupleHeader tup);
-extern CommandId HeapTupleHeaderGetCmax(HeapTupleHeader tup);
-extern void HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
+extern CommandId HeapTupleHeaderGetCmin(const HeapTupleHeaderData *tup);
+extern CommandId HeapTupleHeaderGetCmax(const HeapTupleHeaderData *tup);
+extern void HeapTupleHeaderAdjustCmax(const HeapTupleHeaderData *tup,
 									  CommandId *cmax, bool *iscombo);
 
 /* Prototype for HeapTupleHeader accessors in heapam.c */
-extern TransactionId HeapTupleGetUpdateXid(HeapTupleHeader tuple);
+extern TransactionId HeapTupleGetUpdateXid(const HeapTupleHeaderData *tup);
 
 #endif							/* HTUP_H */

base-commit: d85ce012f99f63249bb45a78fcecb7a2383920b1
-- 
2.47.1


From 89308e0446259a1ae619e39603324d2c2b8ed66c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 27 Dec 2024 11:07:35 +0100
Subject: [PATCH 2/3] Convert macros to static inline functions
 (htup_details.h, itup.h)

---
 contrib/pageinspect/heapfuncs.c   |  15 +-
 src/include/access/htup_details.h | 543 ++++++++++++++++++------------
 src/include/access/itup.h         |  20 +-
 3 files changed, 352 insertions(+), 226 deletions(-)

diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 41ff597199c..4961a3c866c 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -42,13 +42,14 @@
  * was used to upgrade from an older version, tuples might still have an
  * oid. Seems worthwhile to display that.
  */
-#define HeapTupleHeaderGetOidOld(tup) \
-( \
-	((tup)->t_infomask & HEAP_HASOID_OLD) ? \
-	   *((Oid *) ((char *)(tup) + (tup)->t_hoff - sizeof(Oid))) \
-	: \
-		InvalidOid \
-)
+static inline Oid
+HeapTupleHeaderGetOidOld(const HeapTupleHeaderData *tup)
+{
+	if (tup->t_infomask & HEAP_HASOID_OLD)
+		return *((Oid *) ((char *) (tup) + (tup)->t_hoff - sizeof(Oid)));
+	else
+		return InvalidOid;
+}
 
 
 /*
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index 0d1adff540f..bab480fe2f0 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -225,12 +225,13 @@ struct HeapTupleHeaderData
  *
  * See also HeapTupleHeaderIsOnlyLocked, which also checks for a possible
  * aborted updater transaction.
- *
- * Beware of multiple evaluations of the argument.
  */
-#define HEAP_XMAX_IS_LOCKED_ONLY(infomask) \
-	(((infomask) & HEAP_XMAX_LOCK_ONLY) || \
-	 (((infomask) & (HEAP_XMAX_IS_MULTI | HEAP_LOCK_MASK)) == HEAP_XMAX_EXCL_LOCK))
+static inline bool
+HEAP_XMAX_IS_LOCKED_ONLY(uint16 infomask)
+{
+	return (infomask & HEAP_XMAX_LOCK_ONLY) ||
+		(infomask & (HEAP_XMAX_IS_MULTI | HEAP_LOCK_MASK)) == HEAP_XMAX_EXCL_LOCK;
+}
 
 /*
  * A tuple that has HEAP_XMAX_IS_MULTI and HEAP_XMAX_LOCK_ONLY but neither of
@@ -250,22 +251,35 @@ struct HeapTupleHeaderData
  * bogus, regardless of where they stand with respect to the current valid
  * multixact range.
  */
-#define HEAP_LOCKED_UPGRADED(infomask) \
-( \
-	 ((infomask) & HEAP_XMAX_IS_MULTI) != 0 && \
-	 ((infomask) & HEAP_XMAX_LOCK_ONLY) != 0 && \
-	 (((infomask) & (HEAP_XMAX_EXCL_LOCK | HEAP_XMAX_KEYSHR_LOCK)) == 0) \
-)
+static inline bool
+HEAP_LOCKED_UPGRADED(uint16 infomask)
+{
+	return
+		(infomask & HEAP_XMAX_IS_MULTI) != 0 &&
+		(infomask & HEAP_XMAX_LOCK_ONLY) != 0 &&
+		(infomask & (HEAP_XMAX_EXCL_LOCK | HEAP_XMAX_KEYSHR_LOCK)) == 0;
+}
 
 /*
  * Use these to test whether a particular lock is applied to a tuple
  */
-#define HEAP_XMAX_IS_SHR_LOCKED(infomask) \
-	(((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_SHR_LOCK)
-#define HEAP_XMAX_IS_EXCL_LOCKED(infomask) \
-	(((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_EXCL_LOCK)
-#define HEAP_XMAX_IS_KEYSHR_LOCKED(infomask) \
-	(((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_KEYSHR_LOCK)
+static inline bool
+HEAP_XMAX_IS_SHR_LOCKED(int16 infomask)
+{
+	return (infomask & HEAP_LOCK_MASK) == HEAP_XMAX_SHR_LOCK;
+}
+
+static inline bool
+HEAP_XMAX_IS_EXCL_LOCKED(int16 infomask)
+{
+	return (infomask & HEAP_LOCK_MASK) == HEAP_XMAX_EXCL_LOCK;
+}
+
+static inline bool
+HEAP_XMAX_IS_KEYSHR_LOCKED(int16 infomask)
+{
+	return (infomask & HEAP_LOCK_MASK) == HEAP_XMAX_KEYSHR_LOCK;
+}
 
 /* turn these all off when Xmax is to change */
 #define HEAP_XMAX_BITS (HEAP_XMAX_COMMITTED | HEAP_XMAX_INVALID | \
@@ -292,12 +306,11 @@ struct HeapTupleHeaderData
 #define HEAP_TUPLE_HAS_MATCH	HEAP_ONLY_TUPLE /* tuple has a join match */
 
 /*
- * HeapTupleHeader accessor macros
- *
- * Note: beware of multiple evaluations of "tup" argument.  But the Set
- * macros evaluate their other argument only once.
+ * HeapTupleHeader accessor functions
  */
 
+static bool HeapTupleHeaderXminFrozen(const HeapTupleHeaderData *tup);
+
 /*
  * HeapTupleHeaderGetRawXmin returns the "raw" xmin field, which is the xid
  * originally used to insert the tuple.  However, the tuple might actually
@@ -306,56 +319,78 @@ struct HeapTupleHeaderData
  * the xmin to FrozenTransactionId, and that value may still be encountered
  * on disk.
  */
-#define HeapTupleHeaderGetRawXmin(tup) \
-( \
-	(tup)->t_choice.t_heap.t_xmin \
-)
+static inline TransactionId
+HeapTupleHeaderGetRawXmin(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_heap.t_xmin;
+}
 
-#define HeapTupleHeaderGetXmin(tup) \
-( \
-	HeapTupleHeaderXminFrozen(tup) ? \
-		FrozenTransactionId : HeapTupleHeaderGetRawXmin(tup) \
-)
+static inline TransactionId
+HeapTupleHeaderGetXmin(const HeapTupleHeaderData *tup)
+{
+	return HeapTupleHeaderXminFrozen(tup) ?
+		FrozenTransactionId : HeapTupleHeaderGetRawXmin(tup);
+}
 
-#define HeapTupleHeaderSetXmin(tup, xid) \
-( \
-	(tup)->t_choice.t_heap.t_xmin = (xid) \
-)
+static inline void
+HeapTupleHeaderSetXmin(HeapTupleHeaderData *tup, TransactionId xid)
+{
+	tup->t_choice.t_heap.t_xmin = xid;
+}
 
-#define HeapTupleHeaderXminCommitted(tup) \
-( \
-	((tup)->t_infomask & HEAP_XMIN_COMMITTED) != 0 \
-)
+static inline bool
+HeapTupleHeaderXminCommitted(const HeapTupleHeaderData *tup)
+{
+	return (tup->t_infomask & HEAP_XMIN_COMMITTED) != 0;
+}
 
-#define HeapTupleHeaderXminInvalid(tup) \
-( \
-	((tup)->t_infomask & (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID)) == \
-		HEAP_XMIN_INVALID \
-)
+static inline bool
+HeapTupleHeaderXminInvalid(const HeapTupleHeaderData *tup) \
+{
+	return (tup->t_infomask & (HEAP_XMIN_COMMITTED | HEAP_XMIN_INVALID)) ==
+		HEAP_XMIN_INVALID;
+}
 
-#define HeapTupleHeaderXminFrozen(tup) \
-( \
-	((tup)->t_infomask & (HEAP_XMIN_FROZEN)) == HEAP_XMIN_FROZEN \
-)
+static inline bool
+HeapTupleHeaderXminFrozen(const HeapTupleHeaderData *tup)
+{
+	return (tup->t_infomask & HEAP_XMIN_FROZEN) == HEAP_XMIN_FROZEN;
+}
 
-#define HeapTupleHeaderSetXminCommitted(tup) \
-( \
-	AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
-	((tup)->t_infomask |= HEAP_XMIN_COMMITTED) \
-)
+static inline void
+HeapTupleHeaderSetXminCommitted(HeapTupleHeaderData *tup)
+{
+	Assert(!HeapTupleHeaderXminInvalid(tup));
+	tup->t_infomask |= HEAP_XMIN_COMMITTED;
+}
 
-#define HeapTupleHeaderSetXminInvalid(tup) \
-( \
-	AssertMacro(!HeapTupleHeaderXminCommitted(tup)), \
-	((tup)->t_infomask |= HEAP_XMIN_INVALID) \
-)
+static inline void
+HeapTupleHeaderSetXminInvalid(HeapTupleHeaderData *tup)
+{
+	Assert(!HeapTupleHeaderXminCommitted(tup));
+	tup->t_infomask |= HEAP_XMIN_INVALID;
+}
 
-#define HeapTupleHeaderSetXminFrozen(tup) \
-( \
-	AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
-	((tup)->t_infomask |= HEAP_XMIN_FROZEN) \
-)
+static inline void
+HeapTupleHeaderSetXminFrozen(HeapTupleHeaderData *tup)
+{
+	Assert(!HeapTupleHeaderXminInvalid(tup));
+	tup->t_infomask |= HEAP_XMIN_FROZEN;
+}
 
+static inline TransactionId
+HeapTupleHeaderGetRawXmax(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_heap.t_xmax;
+}
+
+static inline void
+HeapTupleHeaderSetXmax(HeapTupleHeaderData *tup, TransactionId xid)
+{
+	tup->t_choice.t_heap.t_xmax = xid;
+}
+
+#ifndef FRONTEND
 /*
  * HeapTupleHeaderGetRawXmax gets you the raw Xmax field.  To find out the Xid
  * that updated a tuple, you might need to resolve the MultiXactId if certain
@@ -363,25 +398,17 @@ struct HeapTupleHeaderData
  * to resolve the MultiXactId if necessary.  This might involve multixact I/O,
  * so it should only be used if absolutely necessary.
  */
-#define HeapTupleHeaderGetUpdateXid(tup) \
-( \
-	(!((tup)->t_infomask & HEAP_XMAX_INVALID) && \
-	 ((tup)->t_infomask & HEAP_XMAX_IS_MULTI) && \
-	 !((tup)->t_infomask & HEAP_XMAX_LOCK_ONLY)) ? \
-		HeapTupleGetUpdateXid(tup) \
-	: \
-		HeapTupleHeaderGetRawXmax(tup) \
-)
-
-#define HeapTupleHeaderGetRawXmax(tup) \
-( \
-	(tup)->t_choice.t_heap.t_xmax \
-)
-
-#define HeapTupleHeaderSetXmax(tup, xid) \
-( \
-	(tup)->t_choice.t_heap.t_xmax = (xid) \
-)
+static inline TransactionId
+HeapTupleHeaderGetUpdateXid(const HeapTupleHeaderData *tup)
+{
+	if (!((tup)->t_infomask & HEAP_XMAX_INVALID) &&
+		((tup)->t_infomask & HEAP_XMAX_IS_MULTI) &&
+		!((tup)->t_infomask & HEAP_XMAX_LOCK_ONLY))
+		return HeapTupleGetUpdateXid(tup);
+	else
+		return HeapTupleHeaderGetRawXmax(tup);
+}
+#endif							/* FRONTEND */
 
 /*
  * HeapTupleHeaderGetRawCommandId will give you what's in the header whether
@@ -389,147 +416,168 @@ struct HeapTupleHeaderData
  * HeapTupleHeaderGetCmax instead, but note that those Assert that you can
  * get a legitimate result, ie you are in the originating transaction!
  */
-#define HeapTupleHeaderGetRawCommandId(tup) \
-( \
-	(tup)->t_choice.t_heap.t_field3.t_cid \
-)
+static inline CommandId
+HeapTupleHeaderGetRawCommandId(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_heap.t_field3.t_cid;
+}
 
 /* SetCmin is reasonably simple since we never need a combo CID */
-#define HeapTupleHeaderSetCmin(tup, cid) \
-do { \
-	Assert(!((tup)->t_infomask & HEAP_MOVED)); \
-	(tup)->t_choice.t_heap.t_field3.t_cid = (cid); \
-	(tup)->t_infomask &= ~HEAP_COMBOCID; \
-} while (0)
+static inline void
+HeapTupleHeaderSetCmin(HeapTupleHeaderData *tup, CommandId cid)
+{
+	Assert(!(tup->t_infomask & HEAP_MOVED));
+	tup->t_choice.t_heap.t_field3.t_cid = cid;
+	tup->t_infomask &= ~HEAP_COMBOCID;
+}
 
 /* SetCmax must be used after HeapTupleHeaderAdjustCmax; see combocid.c */
-#define HeapTupleHeaderSetCmax(tup, cid, iscombo) \
-do { \
-	Assert(!((tup)->t_infomask & HEAP_MOVED)); \
-	(tup)->t_choice.t_heap.t_field3.t_cid = (cid); \
-	if (iscombo) \
-		(tup)->t_infomask |= HEAP_COMBOCID; \
-	else \
-		(tup)->t_infomask &= ~HEAP_COMBOCID; \
-} while (0)
-
-#define HeapTupleHeaderGetXvac(tup) \
-( \
-	((tup)->t_infomask & HEAP_MOVED) ? \
-		(tup)->t_choice.t_heap.t_field3.t_xvac \
-	: \
-		InvalidTransactionId \
-)
+static inline void
+HeapTupleHeaderSetCmax(HeapTupleHeaderData *tup, CommandId cid, bool iscombo)
+{
+	Assert(!((tup)->t_infomask & HEAP_MOVED));
+	tup->t_choice.t_heap.t_field3.t_cid = cid;
+	if (iscombo)
+		tup->t_infomask |= HEAP_COMBOCID;
+	else
+		tup->t_infomask &= ~HEAP_COMBOCID;
+}
+
+static inline TransactionId
+HeapTupleHeaderGetXvac(const HeapTupleHeaderData *tup)
+{
+	if (tup->t_infomask & HEAP_MOVED)
+		return tup->t_choice.t_heap.t_field3.t_xvac;
+	else
+		return InvalidTransactionId;
+}
 
-#define HeapTupleHeaderSetXvac(tup, xid) \
-do { \
-	Assert((tup)->t_infomask & HEAP_MOVED); \
-	(tup)->t_choice.t_heap.t_field3.t_xvac = (xid); \
-} while (0)
+static inline void
+HeapTupleHeaderSetXvac(HeapTupleHeaderData *tup, TransactionId xid)
+{
+	Assert(tup->t_infomask & HEAP_MOVED);
+	tup->t_choice.t_heap.t_field3.t_xvac = xid;
+}
 
 StaticAssertDecl(MaxOffsetNumber < SpecTokenOffsetNumber,
 				 "invalid speculative token constant");
 
-#define HeapTupleHeaderIsSpeculative(tup) \
-( \
-	(ItemPointerGetOffsetNumberNoCheck(&(tup)->t_ctid) == SpecTokenOffsetNumber) \
-)
+static inline bool
+HeapTupleHeaderIsSpeculative(const HeapTupleHeaderData *tup)
+{
+	return ItemPointerGetOffsetNumberNoCheck(&tup->t_ctid) == SpecTokenOffsetNumber;
+}
 
-#define HeapTupleHeaderGetSpeculativeToken(tup) \
-( \
-	AssertMacro(HeapTupleHeaderIsSpeculative(tup)), \
-	ItemPointerGetBlockNumber(&(tup)->t_ctid) \
-)
+static inline BlockNumber
+HeapTupleHeaderGetSpeculativeToken(const HeapTupleHeaderData *tup)
+{
+	Assert(HeapTupleHeaderIsSpeculative(tup));
+	return ItemPointerGetBlockNumber(&tup->t_ctid);
+}
 
-#define HeapTupleHeaderSetSpeculativeToken(tup, token)	\
-( \
-	ItemPointerSet(&(tup)->t_ctid, token, SpecTokenOffsetNumber) \
-)
+static inline void
+HeapTupleHeaderSetSpeculativeToken(HeapTupleHeaderData *tup, BlockNumber token)
+{
+	ItemPointerSet(&tup->t_ctid, token, SpecTokenOffsetNumber);
+}
 
-#define HeapTupleHeaderIndicatesMovedPartitions(tup) \
-	ItemPointerIndicatesMovedPartitions(&(tup)->t_ctid)
+static inline bool
+HeapTupleHeaderIndicatesMovedPartitions(const HeapTupleHeaderData *tup)
+{
+	return ItemPointerIndicatesMovedPartitions(&tup->t_ctid);
+}
 
-#define HeapTupleHeaderSetMovedPartitions(tup) \
-	ItemPointerSetMovedPartitions(&(tup)->t_ctid)
+static inline void
+HeapTupleHeaderSetMovedPartitions(HeapTupleHeaderData *tup)
+{
+	ItemPointerSetMovedPartitions(&tup->t_ctid);
+}
 
-#define HeapTupleHeaderGetDatumLength(tup) \
-	VARSIZE(tup)
+static inline uint32
+HeapTupleHeaderGetDatumLength(const HeapTupleHeaderData *tup)
+{
+	return VARSIZE(tup);
+}
 
-#define HeapTupleHeaderSetDatumLength(tup, len) \
-	SET_VARSIZE(tup, len)
+static inline void
+HeapTupleHeaderSetDatumLength(HeapTupleHeaderData *tup, uint32 len)
+{
+	SET_VARSIZE(tup, len);
+}
 
-#define HeapTupleHeaderGetTypeId(tup) \
-( \
-	(tup)->t_choice.t_datum.datum_typeid \
-)
+static inline Oid
+HeapTupleHeaderGetTypeId(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_datum.datum_typeid;
+}
 
-#define HeapTupleHeaderSetTypeId(tup, typeid) \
-( \
-	(tup)->t_choice.t_datum.datum_typeid = (typeid) \
-)
+static inline void
+HeapTupleHeaderSetTypeId(HeapTupleHeaderData *tup, Oid typeid)
+{
+	tup->t_choice.t_datum.datum_typeid = typeid;
+}
 
-#define HeapTupleHeaderGetTypMod(tup) \
-( \
-	(tup)->t_choice.t_datum.datum_typmod \
-)
+static inline int32
+HeapTupleHeaderGetTypMod(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_datum.datum_typmod;
+}
 
-#define HeapTupleHeaderSetTypMod(tup, typmod) \
-( \
-	(tup)->t_choice.t_datum.datum_typmod = (typmod) \
-)
+static inline void
+HeapTupleHeaderSetTypMod(HeapTupleHeaderData *tup, int32 typmod)
+{
+	tup->t_choice.t_datum.datum_typmod = typmod;
+}
 
 /*
  * Note that we stop considering a tuple HOT-updated as soon as it is known
  * aborted or the would-be updating transaction is known aborted.  For best
- * efficiency, check tuple visibility before using this macro, so that the
+ * efficiency, check tuple visibility before using this function, so that the
  * INVALID bits will be as up to date as possible.
  */
-#define HeapTupleHeaderIsHotUpdated(tup) \
-( \
-	((tup)->t_infomask2 & HEAP_HOT_UPDATED) != 0 && \
-	((tup)->t_infomask & HEAP_XMAX_INVALID) == 0 && \
-	!HeapTupleHeaderXminInvalid(tup) \
-)
-
-#define HeapTupleHeaderSetHotUpdated(tup) \
-( \
-	(tup)->t_infomask2 |= HEAP_HOT_UPDATED \
-)
-
-#define HeapTupleHeaderClearHotUpdated(tup) \
-( \
-	(tup)->t_infomask2 &= ~HEAP_HOT_UPDATED \
-)
+static inline bool
+HeapTupleHeaderIsHotUpdated(const HeapTupleHeaderData *tup)
+{
+	return
+		(tup->t_infomask2 & HEAP_HOT_UPDATED) != 0 &&
+		(tup->t_infomask & HEAP_XMAX_INVALID) == 0 &&
+		!HeapTupleHeaderXminInvalid(tup);
+}
 
-#define HeapTupleHeaderIsHeapOnly(tup) \
-( \
-  ((tup)->t_infomask2 & HEAP_ONLY_TUPLE) != 0 \
-)
+static inline void
+HeapTupleHeaderSetHotUpdated(HeapTupleHeaderData *tup)
+{
+	tup->t_infomask2 |= HEAP_HOT_UPDATED;
+}
 
-#define HeapTupleHeaderSetHeapOnly(tup) \
-( \
-  (tup)->t_infomask2 |= HEAP_ONLY_TUPLE \
-)
+static inline void
+HeapTupleHeaderClearHotUpdated(HeapTupleHeaderData *tup)
+{
+	tup->t_infomask2 &= ~HEAP_HOT_UPDATED;
+}
 
-#define HeapTupleHeaderClearHeapOnly(tup) \
-( \
-  (tup)->t_infomask2 &= ~HEAP_ONLY_TUPLE \
-)
+static inline bool
+HeapTupleHeaderIsHeapOnly(const HeapTupleHeaderData *tup) \
+{
+	return (tup->t_infomask2 & HEAP_ONLY_TUPLE) != 0;
+}
 
-#define HeapTupleHeaderHasMatch(tup) \
-( \
-  ((tup)->t_infomask2 & HEAP_TUPLE_HAS_MATCH) != 0 \
-)
+static inline void
+HeapTupleHeaderSetHeapOnly(HeapTupleHeaderData *tup)
+{
+	tup->t_infomask2 |= HEAP_ONLY_TUPLE;
+}
 
-#define HeapTupleHeaderSetMatch(tup) \
-( \
-  (tup)->t_infomask2 |= HEAP_TUPLE_HAS_MATCH \
-)
+static inline void
+HeapTupleHeaderClearHeapOnly(HeapTupleHeaderData *tup)
+{
+	tup->t_infomask2 &= ~HEAP_ONLY_TUPLE;
+}
 
-#define HeapTupleHeaderClearMatch(tup) \
-( \
-  (tup)->t_infomask2 &= ~HEAP_TUPLE_HAS_MATCH \
-)
+/*
+ * These are used with both HeapTuple and MinimalTuple, so they must be
+ * macros.
+ */
 
 #define HeapTupleHeaderGetNatts(tup) \
 	((tup)->t_infomask2 & HEAP_NATTS_MASK)
@@ -547,7 +595,11 @@ StaticAssertDecl(MaxOffsetNumber < SpecTokenOffsetNumber,
  * BITMAPLEN(NATTS) -
  *		Computes size of null bitmap given number of data columns.
  */
-#define BITMAPLEN(NATTS)	(((int)(NATTS) + 7) / 8)
+static inline int
+BITMAPLEN(int NATTS)
+{
+	return (NATTS + 7) / 8;
+}
 
 /*
  * MaxHeapTupleSize is the maximum allowed size of a heap tuple, including
@@ -651,48 +703,107 @@ struct MinimalTupleData
 
 #define SizeofMinimalTupleHeader offsetof(MinimalTupleData, t_bits)
 
+/*
+ * MinimalTuple accessor functions
+ */
+
+static inline bool
+HeapTupleHeaderHasMatch(const MinimalTupleData *tup)
+{
+	return (tup->t_infomask2 & HEAP_TUPLE_HAS_MATCH) != 0;
+}
+
+static inline void
+HeapTupleHeaderSetMatch(MinimalTupleData *tup)
+{
+	tup->t_infomask2 |= HEAP_TUPLE_HAS_MATCH;
+}
+
+static inline void
+HeapTupleHeaderClearMatch(MinimalTupleData *tup)
+{
+	tup->t_infomask2 &= ~HEAP_TUPLE_HAS_MATCH;
+}
+
 
 /*
  * GETSTRUCT - given a HeapTuple pointer, return address of the user data
  */
-#define GETSTRUCT(TUP) ((char *) ((TUP)->t_data) + (TUP)->t_data->t_hoff)
+static inline void *
+GETSTRUCT(const HeapTupleData *tuple)
+{
+	return ((char *) (tuple->t_data) + tuple->t_data->t_hoff);
+}
 
 /*
- * Accessor macros to be used with HeapTuple pointers.
+ * Accessor functions to be used with HeapTuple pointers.
  */
 
-#define HeapTupleHasNulls(tuple) \
-		(((tuple)->t_data->t_infomask & HEAP_HASNULL) != 0)
+static inline bool
+HeapTupleHasNulls(const HeapTupleData *tuple)
+{
+	return (tuple->t_data->t_infomask & HEAP_HASNULL) != 0;
+}
 
-#define HeapTupleNoNulls(tuple) \
-		(!((tuple)->t_data->t_infomask & HEAP_HASNULL))
+static inline bool
+HeapTupleNoNulls(const HeapTupleData *tuple)
+{
+	return !HeapTupleHasNulls(tuple);
+}
 
-#define HeapTupleHasVarWidth(tuple) \
-		(((tuple)->t_data->t_infomask & HEAP_HASVARWIDTH) != 0)
+static inline bool
+HeapTupleHasVarWidth(const HeapTupleData *tuple)
+{
+	return (tuple->t_data->t_infomask & HEAP_HASVARWIDTH) != 0;
+}
 
-#define HeapTupleAllFixed(tuple) \
-		(!((tuple)->t_data->t_infomask & HEAP_HASVARWIDTH))
+static inline bool
+HeapTupleAllFixed(const HeapTupleData *tuple)
+{
+	return !HeapTupleHasVarWidth(tuple);
+}
 
-#define HeapTupleHasExternal(tuple) \
-		(((tuple)->t_data->t_infomask & HEAP_HASEXTERNAL) != 0)
+static inline bool
+HeapTupleHasExternal(const HeapTupleData *tuple)
+{
+	return (tuple->t_data->t_infomask & HEAP_HASEXTERNAL) != 0;
+}
 
-#define HeapTupleIsHotUpdated(tuple) \
-		HeapTupleHeaderIsHotUpdated((tuple)->t_data)
+static inline bool
+HeapTupleIsHotUpdated(const HeapTupleData *tuple)
+{
+	return HeapTupleHeaderIsHotUpdated(tuple->t_data);
+}
 
-#define HeapTupleSetHotUpdated(tuple) \
-		HeapTupleHeaderSetHotUpdated((tuple)->t_data)
+static inline void
+HeapTupleSetHotUpdated(const HeapTupleData *tuple)
+{
+	HeapTupleHeaderSetHotUpdated(tuple->t_data);
+}
 
-#define HeapTupleClearHotUpdated(tuple) \
-		HeapTupleHeaderClearHotUpdated((tuple)->t_data)
+static inline void
+HeapTupleClearHotUpdated(const HeapTupleData *tuple)
+{
+	HeapTupleHeaderClearHotUpdated(tuple->t_data);
+}
 
-#define HeapTupleIsHeapOnly(tuple) \
-		HeapTupleHeaderIsHeapOnly((tuple)->t_data)
+static inline bool
+HeapTupleIsHeapOnly(const HeapTupleData *tuple)
+{
+	return HeapTupleHeaderIsHeapOnly(tuple->t_data);
+}
 
-#define HeapTupleSetHeapOnly(tuple) \
-		HeapTupleHeaderSetHeapOnly((tuple)->t_data)
+static inline void
+HeapTupleSetHeapOnly(const HeapTupleData *tuple)
+{
+	HeapTupleHeaderSetHeapOnly(tuple->t_data);
+}
 
-#define HeapTupleClearHeapOnly(tuple) \
-		HeapTupleHeaderClearHeapOnly((tuple)->t_data)
+static inline void
+HeapTupleClearHeapOnly(const HeapTupleData *tuple)
+{
+	HeapTupleHeaderClearHeapOnly(tuple->t_data);
+}
 
 /* prototypes for functions in common/heaptuple.c */
 extern Size heap_compute_data_size(TupleDesc tupleDesc,
diff --git a/src/include/access/itup.h b/src/include/access/itup.h
index 4393b19a7fd..bce0e8aa8be 100644
--- a/src/include/access/itup.h
+++ b/src/include/access/itup.h
@@ -68,9 +68,23 @@ typedef IndexAttributeBitMapData * IndexAttributeBitMap;
 #define INDEX_VAR_MASK	0x4000
 #define INDEX_NULL_MASK 0x8000
 
-#define IndexTupleSize(itup)		((Size) ((itup)->t_info & INDEX_SIZE_MASK))
-#define IndexTupleHasNulls(itup)	((((IndexTuple) (itup))->t_info & INDEX_NULL_MASK))
-#define IndexTupleHasVarwidths(itup) ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK))
+static inline Size
+IndexTupleSize(const IndexTupleData *itup)
+{
+	return (itup->t_info & INDEX_SIZE_MASK);
+}
+
+static inline bool
+IndexTupleHasNulls(const IndexTupleData *itup)
+{
+	return itup->t_info & INDEX_NULL_MASK;
+}
+
+static inline bool
+IndexTupleHasVarwidths(const IndexTupleData *itup)
+{
+	return itup->t_info & INDEX_VAR_MASK;
+}
 
 
 /* routines in indextuple.c */
-- 
2.47.1


From b8f2c2f144c1763029ba8b84093a1885f57627a1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 27 Dec 2024 11:07:35 +0100
Subject: [PATCH 3/3] WIP: Use some generic selection

Use generic selection (C11 _Generic) to allow a few functions in
htup_details.h to work with both HeapTuple and MinimalTuple.
---
 src/include/access/htup_details.h | 50 +++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index bab480fe2f0..cb862ff089d 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -574,21 +574,23 @@ HeapTupleHeaderClearHeapOnly(HeapTupleHeaderData *tup)
 	tup->t_infomask2 &= ~HEAP_ONLY_TUPLE;
 }
 
-/*
- * These are used with both HeapTuple and MinimalTuple, so they must be
- * macros.
- */
-
-#define HeapTupleHeaderGetNatts(tup) \
-	((tup)->t_infomask2 & HEAP_NATTS_MASK)
+static inline int
+HeapTupleHeaderGetNatts(const HeapTupleHeaderData *tup)
+{
+	return tup->t_infomask2 & HEAP_NATTS_MASK;
+}
 
-#define HeapTupleHeaderSetNatts(tup, natts) \
-( \
-	(tup)->t_infomask2 = ((tup)->t_infomask2 & ~HEAP_NATTS_MASK) | (natts) \
-)
+static inline void
+HeapTupleHeaderSetNatts(HeapTupleHeaderData *tup, int natts)
+{
+	tup->t_infomask2 = (tup->t_infomask2 & ~HEAP_NATTS_MASK) | natts;
+}
 
-#define HeapTupleHeaderHasExternal(tup) \
-		(((tup)->t_infomask & HEAP_HASEXTERNAL) != 0)
+static inline bool
+HeapTupleHeaderHasExternal(HeapTupleHeaderData *tup)
+{
+	return (tup->t_infomask & HEAP_HASEXTERNAL) != 0;
+}
 
 
 /*
@@ -725,6 +727,28 @@ HeapTupleHeaderClearMatch(MinimalTupleData *tup)
 	tup->t_infomask2 &= ~HEAP_TUPLE_HAS_MATCH;
 }
 
+/*
+ * Accessor functions for both HeapTupleHeader and MinimalTuple
+ */
+
+static inline bool
+MinimalTupleHasExternal(MinimalTupleData *tup)
+{
+	return (tup->t_infomask & HEAP_HASEXTERNAL) != 0;
+}
+
+#define HeapTupleHeaderHasExternal(tup) \
+	_Generic((tup), HeapTupleHeaderData*: HeapTupleHeaderHasExternal, MinimalTupleData*: MinimalTupleHasExternal)(tup)
+
+static inline void
+MinimalTupleSetNatts(MinimalTupleData *tup, int natts)
+{
+	tup->t_infomask2 = (tup->t_infomask2 & ~HEAP_NATTS_MASK) | natts;
+}
+
+#define HeapTupleHeaderSetNatts(tup, natts) \
+	_Generic((tup), HeapTupleHeaderData*: HeapTupleHeaderSetNatts, MinimalTupleData*: MinimalTupleSetNatts)(tup, natts)
+
 
 /*
  * GETSTRUCT - given a HeapTuple pointer, return address of the user data
-- 
2.47.1



Attachments:

  [text/plain] 0001-Add-some-const-decorations-htup.h.patch (3.0K, ../../[email protected]/2-0001-Add-some-const-decorations-htup.h.patch)
  download | inline diff:
From 422181163bfe1a292ee51941a007d54bc127ccec Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 27 Dec 2024 11:07:35 +0100
Subject: [PATCH 1/3] Add some const decorations (htup.h)

---
 src/backend/access/heap/heapam.c  | 6 +++---
 src/backend/utils/time/combocid.c | 6 +++---
 src/include/access/htup.h         | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 329e727f80d..75171e8f2c0 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -7419,10 +7419,10 @@ MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask)
  * checking the hint bits.
  */
 TransactionId
-HeapTupleGetUpdateXid(HeapTupleHeader tuple)
+HeapTupleGetUpdateXid(const HeapTupleHeaderData *tup)
 {
-	return MultiXactIdGetUpdateXid(HeapTupleHeaderGetRawXmax(tuple),
-								   tuple->t_infomask);
+	return MultiXactIdGetUpdateXid(HeapTupleHeaderGetRawXmax(tup),
+								   tup->t_infomask);
 }
 
 /*
diff --git a/src/backend/utils/time/combocid.c b/src/backend/utils/time/combocid.c
index f85510b74ff..1ecdc93b7e2 100644
--- a/src/backend/utils/time/combocid.c
+++ b/src/backend/utils/time/combocid.c
@@ -101,7 +101,7 @@ static CommandId GetRealCmax(CommandId combocid);
  */
 
 CommandId
-HeapTupleHeaderGetCmin(HeapTupleHeader tup)
+HeapTupleHeaderGetCmin(const HeapTupleHeaderData *tup)
 {
 	CommandId	cid = HeapTupleHeaderGetRawCommandId(tup);
 
@@ -115,7 +115,7 @@ HeapTupleHeaderGetCmin(HeapTupleHeader tup)
 }
 
 CommandId
-HeapTupleHeaderGetCmax(HeapTupleHeader tup)
+HeapTupleHeaderGetCmax(const HeapTupleHeaderData *tup)
 {
 	CommandId	cid = HeapTupleHeaderGetRawCommandId(tup);
 
@@ -150,7 +150,7 @@ HeapTupleHeaderGetCmax(HeapTupleHeader tup)
  * changes the tuple in shared buffers.
  */
 void
-HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
+HeapTupleHeaderAdjustCmax(const HeapTupleHeaderData *tup,
 						  CommandId *cmax,
 						  bool *iscombo)
 {
diff --git a/src/include/access/htup.h b/src/include/access/htup.h
index 116cb1bb273..a5f1ef2441d 100644
--- a/src/include/access/htup.h
+++ b/src/include/access/htup.h
@@ -78,12 +78,12 @@ typedef HeapTupleData *HeapTuple;
 #define HeapTupleIsValid(tuple) PointerIsValid(tuple)
 
 /* HeapTupleHeader functions implemented in utils/time/combocid.c */
-extern CommandId HeapTupleHeaderGetCmin(HeapTupleHeader tup);
-extern CommandId HeapTupleHeaderGetCmax(HeapTupleHeader tup);
-extern void HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
+extern CommandId HeapTupleHeaderGetCmin(const HeapTupleHeaderData *tup);
+extern CommandId HeapTupleHeaderGetCmax(const HeapTupleHeaderData *tup);
+extern void HeapTupleHeaderAdjustCmax(const HeapTupleHeaderData *tup,
 									  CommandId *cmax, bool *iscombo);
 
 /* Prototype for HeapTupleHeader accessors in heapam.c */
-extern TransactionId HeapTupleGetUpdateXid(HeapTupleHeader tuple);
+extern TransactionId HeapTupleGetUpdateXid(const HeapTupleHeaderData *tup);
 
 #endif							/* HTUP_H */

base-commit: d85ce012f99f63249bb45a78fcecb7a2383920b1
-- 
2.47.1



  [text/plain] 0002-Convert-macros-to-static-inline-functions-htup_detai.patch (20.3K, ../../[email protected]/3-0002-Convert-macros-to-static-inline-functions-htup_detai.patch)
  download | inline diff:
From 89308e0446259a1ae619e39603324d2c2b8ed66c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 27 Dec 2024 11:07:35 +0100
Subject: [PATCH 2/3] Convert macros to static inline functions
 (htup_details.h, itup.h)

---
 contrib/pageinspect/heapfuncs.c   |  15 +-
 src/include/access/htup_details.h | 543 ++++++++++++++++++------------
 src/include/access/itup.h         |  20 +-
 3 files changed, 352 insertions(+), 226 deletions(-)

diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 41ff597199c..4961a3c866c 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -42,13 +42,14 @@
  * was used to upgrade from an older version, tuples might still have an
  * oid. Seems worthwhile to display that.
  */
-#define HeapTupleHeaderGetOidOld(tup) \
-( \
-	((tup)->t_infomask & HEAP_HASOID_OLD) ? \
-	   *((Oid *) ((char *)(tup) + (tup)->t_hoff - sizeof(Oid))) \
-	: \
-		InvalidOid \
-)
+static inline Oid
+HeapTupleHeaderGetOidOld(const HeapTupleHeaderData *tup)
+{
+	if (tup->t_infomask & HEAP_HASOID_OLD)
+		return *((Oid *) ((char *) (tup) + (tup)->t_hoff - sizeof(Oid)));
+	else
+		return InvalidOid;
+}
 
 
 /*
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index 0d1adff540f..bab480fe2f0 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -225,12 +225,13 @@ struct HeapTupleHeaderData
  *
  * See also HeapTupleHeaderIsOnlyLocked, which also checks for a possible
  * aborted updater transaction.
- *
- * Beware of multiple evaluations of the argument.
  */
-#define HEAP_XMAX_IS_LOCKED_ONLY(infomask) \
-	(((infomask) & HEAP_XMAX_LOCK_ONLY) || \
-	 (((infomask) & (HEAP_XMAX_IS_MULTI | HEAP_LOCK_MASK)) == HEAP_XMAX_EXCL_LOCK))
+static inline bool
+HEAP_XMAX_IS_LOCKED_ONLY(uint16 infomask)
+{
+	return (infomask & HEAP_XMAX_LOCK_ONLY) ||
+		(infomask & (HEAP_XMAX_IS_MULTI | HEAP_LOCK_MASK)) == HEAP_XMAX_EXCL_LOCK;
+}
 
 /*
  * A tuple that has HEAP_XMAX_IS_MULTI and HEAP_XMAX_LOCK_ONLY but neither of
@@ -250,22 +251,35 @@ struct HeapTupleHeaderData
  * bogus, regardless of where they stand with respect to the current valid
  * multixact range.
  */
-#define HEAP_LOCKED_UPGRADED(infomask) \
-( \
-	 ((infomask) & HEAP_XMAX_IS_MULTI) != 0 && \
-	 ((infomask) & HEAP_XMAX_LOCK_ONLY) != 0 && \
-	 (((infomask) & (HEAP_XMAX_EXCL_LOCK | HEAP_XMAX_KEYSHR_LOCK)) == 0) \
-)
+static inline bool
+HEAP_LOCKED_UPGRADED(uint16 infomask)
+{
+	return
+		(infomask & HEAP_XMAX_IS_MULTI) != 0 &&
+		(infomask & HEAP_XMAX_LOCK_ONLY) != 0 &&
+		(infomask & (HEAP_XMAX_EXCL_LOCK | HEAP_XMAX_KEYSHR_LOCK)) == 0;
+}
 
 /*
  * Use these to test whether a particular lock is applied to a tuple
  */
-#define HEAP_XMAX_IS_SHR_LOCKED(infomask) \
-	(((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_SHR_LOCK)
-#define HEAP_XMAX_IS_EXCL_LOCKED(infomask) \
-	(((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_EXCL_LOCK)
-#define HEAP_XMAX_IS_KEYSHR_LOCKED(infomask) \
-	(((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_KEYSHR_LOCK)
+static inline bool
+HEAP_XMAX_IS_SHR_LOCKED(int16 infomask)
+{
+	return (infomask & HEAP_LOCK_MASK) == HEAP_XMAX_SHR_LOCK;
+}
+
+static inline bool
+HEAP_XMAX_IS_EXCL_LOCKED(int16 infomask)
+{
+	return (infomask & HEAP_LOCK_MASK) == HEAP_XMAX_EXCL_LOCK;
+}
+
+static inline bool
+HEAP_XMAX_IS_KEYSHR_LOCKED(int16 infomask)
+{
+	return (infomask & HEAP_LOCK_MASK) == HEAP_XMAX_KEYSHR_LOCK;
+}
 
 /* turn these all off when Xmax is to change */
 #define HEAP_XMAX_BITS (HEAP_XMAX_COMMITTED | HEAP_XMAX_INVALID | \
@@ -292,12 +306,11 @@ struct HeapTupleHeaderData
 #define HEAP_TUPLE_HAS_MATCH	HEAP_ONLY_TUPLE /* tuple has a join match */
 
 /*
- * HeapTupleHeader accessor macros
- *
- * Note: beware of multiple evaluations of "tup" argument.  But the Set
- * macros evaluate their other argument only once.
+ * HeapTupleHeader accessor functions
  */
 
+static bool HeapTupleHeaderXminFrozen(const HeapTupleHeaderData *tup);
+
 /*
  * HeapTupleHeaderGetRawXmin returns the "raw" xmin field, which is the xid
  * originally used to insert the tuple.  However, the tuple might actually
@@ -306,56 +319,78 @@ struct HeapTupleHeaderData
  * the xmin to FrozenTransactionId, and that value may still be encountered
  * on disk.
  */
-#define HeapTupleHeaderGetRawXmin(tup) \
-( \
-	(tup)->t_choice.t_heap.t_xmin \
-)
+static inline TransactionId
+HeapTupleHeaderGetRawXmin(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_heap.t_xmin;
+}
 
-#define HeapTupleHeaderGetXmin(tup) \
-( \
-	HeapTupleHeaderXminFrozen(tup) ? \
-		FrozenTransactionId : HeapTupleHeaderGetRawXmin(tup) \
-)
+static inline TransactionId
+HeapTupleHeaderGetXmin(const HeapTupleHeaderData *tup)
+{
+	return HeapTupleHeaderXminFrozen(tup) ?
+		FrozenTransactionId : HeapTupleHeaderGetRawXmin(tup);
+}
 
-#define HeapTupleHeaderSetXmin(tup, xid) \
-( \
-	(tup)->t_choice.t_heap.t_xmin = (xid) \
-)
+static inline void
+HeapTupleHeaderSetXmin(HeapTupleHeaderData *tup, TransactionId xid)
+{
+	tup->t_choice.t_heap.t_xmin = xid;
+}
 
-#define HeapTupleHeaderXminCommitted(tup) \
-( \
-	((tup)->t_infomask & HEAP_XMIN_COMMITTED) != 0 \
-)
+static inline bool
+HeapTupleHeaderXminCommitted(const HeapTupleHeaderData *tup)
+{
+	return (tup->t_infomask & HEAP_XMIN_COMMITTED) != 0;
+}
 
-#define HeapTupleHeaderXminInvalid(tup) \
-( \
-	((tup)->t_infomask & (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID)) == \
-		HEAP_XMIN_INVALID \
-)
+static inline bool
+HeapTupleHeaderXminInvalid(const HeapTupleHeaderData *tup) \
+{
+	return (tup->t_infomask & (HEAP_XMIN_COMMITTED | HEAP_XMIN_INVALID)) ==
+		HEAP_XMIN_INVALID;
+}
 
-#define HeapTupleHeaderXminFrozen(tup) \
-( \
-	((tup)->t_infomask & (HEAP_XMIN_FROZEN)) == HEAP_XMIN_FROZEN \
-)
+static inline bool
+HeapTupleHeaderXminFrozen(const HeapTupleHeaderData *tup)
+{
+	return (tup->t_infomask & HEAP_XMIN_FROZEN) == HEAP_XMIN_FROZEN;
+}
 
-#define HeapTupleHeaderSetXminCommitted(tup) \
-( \
-	AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
-	((tup)->t_infomask |= HEAP_XMIN_COMMITTED) \
-)
+static inline void
+HeapTupleHeaderSetXminCommitted(HeapTupleHeaderData *tup)
+{
+	Assert(!HeapTupleHeaderXminInvalid(tup));
+	tup->t_infomask |= HEAP_XMIN_COMMITTED;
+}
 
-#define HeapTupleHeaderSetXminInvalid(tup) \
-( \
-	AssertMacro(!HeapTupleHeaderXminCommitted(tup)), \
-	((tup)->t_infomask |= HEAP_XMIN_INVALID) \
-)
+static inline void
+HeapTupleHeaderSetXminInvalid(HeapTupleHeaderData *tup)
+{
+	Assert(!HeapTupleHeaderXminCommitted(tup));
+	tup->t_infomask |= HEAP_XMIN_INVALID;
+}
 
-#define HeapTupleHeaderSetXminFrozen(tup) \
-( \
-	AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
-	((tup)->t_infomask |= HEAP_XMIN_FROZEN) \
-)
+static inline void
+HeapTupleHeaderSetXminFrozen(HeapTupleHeaderData *tup)
+{
+	Assert(!HeapTupleHeaderXminInvalid(tup));
+	tup->t_infomask |= HEAP_XMIN_FROZEN;
+}
 
+static inline TransactionId
+HeapTupleHeaderGetRawXmax(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_heap.t_xmax;
+}
+
+static inline void
+HeapTupleHeaderSetXmax(HeapTupleHeaderData *tup, TransactionId xid)
+{
+	tup->t_choice.t_heap.t_xmax = xid;
+}
+
+#ifndef FRONTEND
 /*
  * HeapTupleHeaderGetRawXmax gets you the raw Xmax field.  To find out the Xid
  * that updated a tuple, you might need to resolve the MultiXactId if certain
@@ -363,25 +398,17 @@ struct HeapTupleHeaderData
  * to resolve the MultiXactId if necessary.  This might involve multixact I/O,
  * so it should only be used if absolutely necessary.
  */
-#define HeapTupleHeaderGetUpdateXid(tup) \
-( \
-	(!((tup)->t_infomask & HEAP_XMAX_INVALID) && \
-	 ((tup)->t_infomask & HEAP_XMAX_IS_MULTI) && \
-	 !((tup)->t_infomask & HEAP_XMAX_LOCK_ONLY)) ? \
-		HeapTupleGetUpdateXid(tup) \
-	: \
-		HeapTupleHeaderGetRawXmax(tup) \
-)
-
-#define HeapTupleHeaderGetRawXmax(tup) \
-( \
-	(tup)->t_choice.t_heap.t_xmax \
-)
-
-#define HeapTupleHeaderSetXmax(tup, xid) \
-( \
-	(tup)->t_choice.t_heap.t_xmax = (xid) \
-)
+static inline TransactionId
+HeapTupleHeaderGetUpdateXid(const HeapTupleHeaderData *tup)
+{
+	if (!((tup)->t_infomask & HEAP_XMAX_INVALID) &&
+		((tup)->t_infomask & HEAP_XMAX_IS_MULTI) &&
+		!((tup)->t_infomask & HEAP_XMAX_LOCK_ONLY))
+		return HeapTupleGetUpdateXid(tup);
+	else
+		return HeapTupleHeaderGetRawXmax(tup);
+}
+#endif							/* FRONTEND */
 
 /*
  * HeapTupleHeaderGetRawCommandId will give you what's in the header whether
@@ -389,147 +416,168 @@ struct HeapTupleHeaderData
  * HeapTupleHeaderGetCmax instead, but note that those Assert that you can
  * get a legitimate result, ie you are in the originating transaction!
  */
-#define HeapTupleHeaderGetRawCommandId(tup) \
-( \
-	(tup)->t_choice.t_heap.t_field3.t_cid \
-)
+static inline CommandId
+HeapTupleHeaderGetRawCommandId(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_heap.t_field3.t_cid;
+}
 
 /* SetCmin is reasonably simple since we never need a combo CID */
-#define HeapTupleHeaderSetCmin(tup, cid) \
-do { \
-	Assert(!((tup)->t_infomask & HEAP_MOVED)); \
-	(tup)->t_choice.t_heap.t_field3.t_cid = (cid); \
-	(tup)->t_infomask &= ~HEAP_COMBOCID; \
-} while (0)
+static inline void
+HeapTupleHeaderSetCmin(HeapTupleHeaderData *tup, CommandId cid)
+{
+	Assert(!(tup->t_infomask & HEAP_MOVED));
+	tup->t_choice.t_heap.t_field3.t_cid = cid;
+	tup->t_infomask &= ~HEAP_COMBOCID;
+}
 
 /* SetCmax must be used after HeapTupleHeaderAdjustCmax; see combocid.c */
-#define HeapTupleHeaderSetCmax(tup, cid, iscombo) \
-do { \
-	Assert(!((tup)->t_infomask & HEAP_MOVED)); \
-	(tup)->t_choice.t_heap.t_field3.t_cid = (cid); \
-	if (iscombo) \
-		(tup)->t_infomask |= HEAP_COMBOCID; \
-	else \
-		(tup)->t_infomask &= ~HEAP_COMBOCID; \
-} while (0)
-
-#define HeapTupleHeaderGetXvac(tup) \
-( \
-	((tup)->t_infomask & HEAP_MOVED) ? \
-		(tup)->t_choice.t_heap.t_field3.t_xvac \
-	: \
-		InvalidTransactionId \
-)
+static inline void
+HeapTupleHeaderSetCmax(HeapTupleHeaderData *tup, CommandId cid, bool iscombo)
+{
+	Assert(!((tup)->t_infomask & HEAP_MOVED));
+	tup->t_choice.t_heap.t_field3.t_cid = cid;
+	if (iscombo)
+		tup->t_infomask |= HEAP_COMBOCID;
+	else
+		tup->t_infomask &= ~HEAP_COMBOCID;
+}
+
+static inline TransactionId
+HeapTupleHeaderGetXvac(const HeapTupleHeaderData *tup)
+{
+	if (tup->t_infomask & HEAP_MOVED)
+		return tup->t_choice.t_heap.t_field3.t_xvac;
+	else
+		return InvalidTransactionId;
+}
 
-#define HeapTupleHeaderSetXvac(tup, xid) \
-do { \
-	Assert((tup)->t_infomask & HEAP_MOVED); \
-	(tup)->t_choice.t_heap.t_field3.t_xvac = (xid); \
-} while (0)
+static inline void
+HeapTupleHeaderSetXvac(HeapTupleHeaderData *tup, TransactionId xid)
+{
+	Assert(tup->t_infomask & HEAP_MOVED);
+	tup->t_choice.t_heap.t_field3.t_xvac = xid;
+}
 
 StaticAssertDecl(MaxOffsetNumber < SpecTokenOffsetNumber,
 				 "invalid speculative token constant");
 
-#define HeapTupleHeaderIsSpeculative(tup) \
-( \
-	(ItemPointerGetOffsetNumberNoCheck(&(tup)->t_ctid) == SpecTokenOffsetNumber) \
-)
+static inline bool
+HeapTupleHeaderIsSpeculative(const HeapTupleHeaderData *tup)
+{
+	return ItemPointerGetOffsetNumberNoCheck(&tup->t_ctid) == SpecTokenOffsetNumber;
+}
 
-#define HeapTupleHeaderGetSpeculativeToken(tup) \
-( \
-	AssertMacro(HeapTupleHeaderIsSpeculative(tup)), \
-	ItemPointerGetBlockNumber(&(tup)->t_ctid) \
-)
+static inline BlockNumber
+HeapTupleHeaderGetSpeculativeToken(const HeapTupleHeaderData *tup)
+{
+	Assert(HeapTupleHeaderIsSpeculative(tup));
+	return ItemPointerGetBlockNumber(&tup->t_ctid);
+}
 
-#define HeapTupleHeaderSetSpeculativeToken(tup, token)	\
-( \
-	ItemPointerSet(&(tup)->t_ctid, token, SpecTokenOffsetNumber) \
-)
+static inline void
+HeapTupleHeaderSetSpeculativeToken(HeapTupleHeaderData *tup, BlockNumber token)
+{
+	ItemPointerSet(&tup->t_ctid, token, SpecTokenOffsetNumber);
+}
 
-#define HeapTupleHeaderIndicatesMovedPartitions(tup) \
-	ItemPointerIndicatesMovedPartitions(&(tup)->t_ctid)
+static inline bool
+HeapTupleHeaderIndicatesMovedPartitions(const HeapTupleHeaderData *tup)
+{
+	return ItemPointerIndicatesMovedPartitions(&tup->t_ctid);
+}
 
-#define HeapTupleHeaderSetMovedPartitions(tup) \
-	ItemPointerSetMovedPartitions(&(tup)->t_ctid)
+static inline void
+HeapTupleHeaderSetMovedPartitions(HeapTupleHeaderData *tup)
+{
+	ItemPointerSetMovedPartitions(&tup->t_ctid);
+}
 
-#define HeapTupleHeaderGetDatumLength(tup) \
-	VARSIZE(tup)
+static inline uint32
+HeapTupleHeaderGetDatumLength(const HeapTupleHeaderData *tup)
+{
+	return VARSIZE(tup);
+}
 
-#define HeapTupleHeaderSetDatumLength(tup, len) \
-	SET_VARSIZE(tup, len)
+static inline void
+HeapTupleHeaderSetDatumLength(HeapTupleHeaderData *tup, uint32 len)
+{
+	SET_VARSIZE(tup, len);
+}
 
-#define HeapTupleHeaderGetTypeId(tup) \
-( \
-	(tup)->t_choice.t_datum.datum_typeid \
-)
+static inline Oid
+HeapTupleHeaderGetTypeId(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_datum.datum_typeid;
+}
 
-#define HeapTupleHeaderSetTypeId(tup, typeid) \
-( \
-	(tup)->t_choice.t_datum.datum_typeid = (typeid) \
-)
+static inline void
+HeapTupleHeaderSetTypeId(HeapTupleHeaderData *tup, Oid typeid)
+{
+	tup->t_choice.t_datum.datum_typeid = typeid;
+}
 
-#define HeapTupleHeaderGetTypMod(tup) \
-( \
-	(tup)->t_choice.t_datum.datum_typmod \
-)
+static inline int32
+HeapTupleHeaderGetTypMod(const HeapTupleHeaderData *tup)
+{
+	return tup->t_choice.t_datum.datum_typmod;
+}
 
-#define HeapTupleHeaderSetTypMod(tup, typmod) \
-( \
-	(tup)->t_choice.t_datum.datum_typmod = (typmod) \
-)
+static inline void
+HeapTupleHeaderSetTypMod(HeapTupleHeaderData *tup, int32 typmod)
+{
+	tup->t_choice.t_datum.datum_typmod = typmod;
+}
 
 /*
  * Note that we stop considering a tuple HOT-updated as soon as it is known
  * aborted or the would-be updating transaction is known aborted.  For best
- * efficiency, check tuple visibility before using this macro, so that the
+ * efficiency, check tuple visibility before using this function, so that the
  * INVALID bits will be as up to date as possible.
  */
-#define HeapTupleHeaderIsHotUpdated(tup) \
-( \
-	((tup)->t_infomask2 & HEAP_HOT_UPDATED) != 0 && \
-	((tup)->t_infomask & HEAP_XMAX_INVALID) == 0 && \
-	!HeapTupleHeaderXminInvalid(tup) \
-)
-
-#define HeapTupleHeaderSetHotUpdated(tup) \
-( \
-	(tup)->t_infomask2 |= HEAP_HOT_UPDATED \
-)
-
-#define HeapTupleHeaderClearHotUpdated(tup) \
-( \
-	(tup)->t_infomask2 &= ~HEAP_HOT_UPDATED \
-)
+static inline bool
+HeapTupleHeaderIsHotUpdated(const HeapTupleHeaderData *tup)
+{
+	return
+		(tup->t_infomask2 & HEAP_HOT_UPDATED) != 0 &&
+		(tup->t_infomask & HEAP_XMAX_INVALID) == 0 &&
+		!HeapTupleHeaderXminInvalid(tup);
+}
 
-#define HeapTupleHeaderIsHeapOnly(tup) \
-( \
-  ((tup)->t_infomask2 & HEAP_ONLY_TUPLE) != 0 \
-)
+static inline void
+HeapTupleHeaderSetHotUpdated(HeapTupleHeaderData *tup)
+{
+	tup->t_infomask2 |= HEAP_HOT_UPDATED;
+}
 
-#define HeapTupleHeaderSetHeapOnly(tup) \
-( \
-  (tup)->t_infomask2 |= HEAP_ONLY_TUPLE \
-)
+static inline void
+HeapTupleHeaderClearHotUpdated(HeapTupleHeaderData *tup)
+{
+	tup->t_infomask2 &= ~HEAP_HOT_UPDATED;
+}
 
-#define HeapTupleHeaderClearHeapOnly(tup) \
-( \
-  (tup)->t_infomask2 &= ~HEAP_ONLY_TUPLE \
-)
+static inline bool
+HeapTupleHeaderIsHeapOnly(const HeapTupleHeaderData *tup) \
+{
+	return (tup->t_infomask2 & HEAP_ONLY_TUPLE) != 0;
+}
 
-#define HeapTupleHeaderHasMatch(tup) \
-( \
-  ((tup)->t_infomask2 & HEAP_TUPLE_HAS_MATCH) != 0 \
-)
+static inline void
+HeapTupleHeaderSetHeapOnly(HeapTupleHeaderData *tup)
+{
+	tup->t_infomask2 |= HEAP_ONLY_TUPLE;
+}
 
-#define HeapTupleHeaderSetMatch(tup) \
-( \
-  (tup)->t_infomask2 |= HEAP_TUPLE_HAS_MATCH \
-)
+static inline void
+HeapTupleHeaderClearHeapOnly(HeapTupleHeaderData *tup)
+{
+	tup->t_infomask2 &= ~HEAP_ONLY_TUPLE;
+}
 
-#define HeapTupleHeaderClearMatch(tup) \
-( \
-  (tup)->t_infomask2 &= ~HEAP_TUPLE_HAS_MATCH \
-)
+/*
+ * These are used with both HeapTuple and MinimalTuple, so they must be
+ * macros.
+ */
 
 #define HeapTupleHeaderGetNatts(tup) \
 	((tup)->t_infomask2 & HEAP_NATTS_MASK)
@@ -547,7 +595,11 @@ StaticAssertDecl(MaxOffsetNumber < SpecTokenOffsetNumber,
  * BITMAPLEN(NATTS) -
  *		Computes size of null bitmap given number of data columns.
  */
-#define BITMAPLEN(NATTS)	(((int)(NATTS) + 7) / 8)
+static inline int
+BITMAPLEN(int NATTS)
+{
+	return (NATTS + 7) / 8;
+}
 
 /*
  * MaxHeapTupleSize is the maximum allowed size of a heap tuple, including
@@ -651,48 +703,107 @@ struct MinimalTupleData
 
 #define SizeofMinimalTupleHeader offsetof(MinimalTupleData, t_bits)
 
+/*
+ * MinimalTuple accessor functions
+ */
+
+static inline bool
+HeapTupleHeaderHasMatch(const MinimalTupleData *tup)
+{
+	return (tup->t_infomask2 & HEAP_TUPLE_HAS_MATCH) != 0;
+}
+
+static inline void
+HeapTupleHeaderSetMatch(MinimalTupleData *tup)
+{
+	tup->t_infomask2 |= HEAP_TUPLE_HAS_MATCH;
+}
+
+static inline void
+HeapTupleHeaderClearMatch(MinimalTupleData *tup)
+{
+	tup->t_infomask2 &= ~HEAP_TUPLE_HAS_MATCH;
+}
+
 
 /*
  * GETSTRUCT - given a HeapTuple pointer, return address of the user data
  */
-#define GETSTRUCT(TUP) ((char *) ((TUP)->t_data) + (TUP)->t_data->t_hoff)
+static inline void *
+GETSTRUCT(const HeapTupleData *tuple)
+{
+	return ((char *) (tuple->t_data) + tuple->t_data->t_hoff);
+}
 
 /*
- * Accessor macros to be used with HeapTuple pointers.
+ * Accessor functions to be used with HeapTuple pointers.
  */
 
-#define HeapTupleHasNulls(tuple) \
-		(((tuple)->t_data->t_infomask & HEAP_HASNULL) != 0)
+static inline bool
+HeapTupleHasNulls(const HeapTupleData *tuple)
+{
+	return (tuple->t_data->t_infomask & HEAP_HASNULL) != 0;
+}
 
-#define HeapTupleNoNulls(tuple) \
-		(!((tuple)->t_data->t_infomask & HEAP_HASNULL))
+static inline bool
+HeapTupleNoNulls(const HeapTupleData *tuple)
+{
+	return !HeapTupleHasNulls(tuple);
+}
 
-#define HeapTupleHasVarWidth(tuple) \
-		(((tuple)->t_data->t_infomask & HEAP_HASVARWIDTH) != 0)
+static inline bool
+HeapTupleHasVarWidth(const HeapTupleData *tuple)
+{
+	return (tuple->t_data->t_infomask & HEAP_HASVARWIDTH) != 0;
+}
 
-#define HeapTupleAllFixed(tuple) \
-		(!((tuple)->t_data->t_infomask & HEAP_HASVARWIDTH))
+static inline bool
+HeapTupleAllFixed(const HeapTupleData *tuple)
+{
+	return !HeapTupleHasVarWidth(tuple);
+}
 
-#define HeapTupleHasExternal(tuple) \
-		(((tuple)->t_data->t_infomask & HEAP_HASEXTERNAL) != 0)
+static inline bool
+HeapTupleHasExternal(const HeapTupleData *tuple)
+{
+	return (tuple->t_data->t_infomask & HEAP_HASEXTERNAL) != 0;
+}
 
-#define HeapTupleIsHotUpdated(tuple) \
-		HeapTupleHeaderIsHotUpdated((tuple)->t_data)
+static inline bool
+HeapTupleIsHotUpdated(const HeapTupleData *tuple)
+{
+	return HeapTupleHeaderIsHotUpdated(tuple->t_data);
+}
 
-#define HeapTupleSetHotUpdated(tuple) \
-		HeapTupleHeaderSetHotUpdated((tuple)->t_data)
+static inline void
+HeapTupleSetHotUpdated(const HeapTupleData *tuple)
+{
+	HeapTupleHeaderSetHotUpdated(tuple->t_data);
+}
 
-#define HeapTupleClearHotUpdated(tuple) \
-		HeapTupleHeaderClearHotUpdated((tuple)->t_data)
+static inline void
+HeapTupleClearHotUpdated(const HeapTupleData *tuple)
+{
+	HeapTupleHeaderClearHotUpdated(tuple->t_data);
+}
 
-#define HeapTupleIsHeapOnly(tuple) \
-		HeapTupleHeaderIsHeapOnly((tuple)->t_data)
+static inline bool
+HeapTupleIsHeapOnly(const HeapTupleData *tuple)
+{
+	return HeapTupleHeaderIsHeapOnly(tuple->t_data);
+}
 
-#define HeapTupleSetHeapOnly(tuple) \
-		HeapTupleHeaderSetHeapOnly((tuple)->t_data)
+static inline void
+HeapTupleSetHeapOnly(const HeapTupleData *tuple)
+{
+	HeapTupleHeaderSetHeapOnly(tuple->t_data);
+}
 
-#define HeapTupleClearHeapOnly(tuple) \
-		HeapTupleHeaderClearHeapOnly((tuple)->t_data)
+static inline void
+HeapTupleClearHeapOnly(const HeapTupleData *tuple)
+{
+	HeapTupleHeaderClearHeapOnly(tuple->t_data);
+}
 
 /* prototypes for functions in common/heaptuple.c */
 extern Size heap_compute_data_size(TupleDesc tupleDesc,
diff --git a/src/include/access/itup.h b/src/include/access/itup.h
index 4393b19a7fd..bce0e8aa8be 100644
--- a/src/include/access/itup.h
+++ b/src/include/access/itup.h
@@ -68,9 +68,23 @@ typedef IndexAttributeBitMapData * IndexAttributeBitMap;
 #define INDEX_VAR_MASK	0x4000
 #define INDEX_NULL_MASK 0x8000
 
-#define IndexTupleSize(itup)		((Size) ((itup)->t_info & INDEX_SIZE_MASK))
-#define IndexTupleHasNulls(itup)	((((IndexTuple) (itup))->t_info & INDEX_NULL_MASK))
-#define IndexTupleHasVarwidths(itup) ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK))
+static inline Size
+IndexTupleSize(const IndexTupleData *itup)
+{
+	return (itup->t_info & INDEX_SIZE_MASK);
+}
+
+static inline bool
+IndexTupleHasNulls(const IndexTupleData *itup)
+{
+	return itup->t_info & INDEX_NULL_MASK;
+}
+
+static inline bool
+IndexTupleHasVarwidths(const IndexTupleData *itup)
+{
+	return itup->t_info & INDEX_VAR_MASK;
+}
 
 
 /* routines in indextuple.c */
-- 
2.47.1



  [text/plain] 0003-WIP-Use-some-generic-selection.patch (2.5K, ../../[email protected]/4-0003-WIP-Use-some-generic-selection.patch)
  download | inline diff:
From b8f2c2f144c1763029ba8b84093a1885f57627a1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 27 Dec 2024 11:07:35 +0100
Subject: [PATCH 3/3] WIP: Use some generic selection

Use generic selection (C11 _Generic) to allow a few functions in
htup_details.h to work with both HeapTuple and MinimalTuple.
---
 src/include/access/htup_details.h | 50 +++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index bab480fe2f0..cb862ff089d 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -574,21 +574,23 @@ HeapTupleHeaderClearHeapOnly(HeapTupleHeaderData *tup)
 	tup->t_infomask2 &= ~HEAP_ONLY_TUPLE;
 }
 
-/*
- * These are used with both HeapTuple and MinimalTuple, so they must be
- * macros.
- */
-
-#define HeapTupleHeaderGetNatts(tup) \
-	((tup)->t_infomask2 & HEAP_NATTS_MASK)
+static inline int
+HeapTupleHeaderGetNatts(const HeapTupleHeaderData *tup)
+{
+	return tup->t_infomask2 & HEAP_NATTS_MASK;
+}
 
-#define HeapTupleHeaderSetNatts(tup, natts) \
-( \
-	(tup)->t_infomask2 = ((tup)->t_infomask2 & ~HEAP_NATTS_MASK) | (natts) \
-)
+static inline void
+HeapTupleHeaderSetNatts(HeapTupleHeaderData *tup, int natts)
+{
+	tup->t_infomask2 = (tup->t_infomask2 & ~HEAP_NATTS_MASK) | natts;
+}
 
-#define HeapTupleHeaderHasExternal(tup) \
-		(((tup)->t_infomask & HEAP_HASEXTERNAL) != 0)
+static inline bool
+HeapTupleHeaderHasExternal(HeapTupleHeaderData *tup)
+{
+	return (tup->t_infomask & HEAP_HASEXTERNAL) != 0;
+}
 
 
 /*
@@ -725,6 +727,28 @@ HeapTupleHeaderClearMatch(MinimalTupleData *tup)
 	tup->t_infomask2 &= ~HEAP_TUPLE_HAS_MATCH;
 }
 
+/*
+ * Accessor functions for both HeapTupleHeader and MinimalTuple
+ */
+
+static inline bool
+MinimalTupleHasExternal(MinimalTupleData *tup)
+{
+	return (tup->t_infomask & HEAP_HASEXTERNAL) != 0;
+}
+
+#define HeapTupleHeaderHasExternal(tup) \
+	_Generic((tup), HeapTupleHeaderData*: HeapTupleHeaderHasExternal, MinimalTupleData*: MinimalTupleHasExternal)(tup)
+
+static inline void
+MinimalTupleSetNatts(MinimalTupleData *tup, int natts)
+{
+	tup->t_infomask2 = (tup->t_infomask2 & ~HEAP_NATTS_MASK) | natts;
+}
+
+#define HeapTupleHeaderSetNatts(tup, natts) \
+	_Generic((tup), HeapTupleHeaderData*: HeapTupleHeaderSetNatts, MinimalTupleData*: MinimalTupleSetNatts)(tup, natts)
+
 
 /*
  * GETSTRUCT - given a HeapTuple pointer, return address of the user data
-- 
2.47.1



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


end of thread, other threads:[~2024-12-27 10:16 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18 16:19 [PATCH v41 1/2] Subscripting for jsonb Dmitrii Dolgov <[email protected]>
2024-12-27 10:16 Re: Convert macros to static inline functions Peter Eisentraut <[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