Add more commentaries in core

Move up `jsonb_set_element` close to `jsonb_get_element`

Author: erthalion

src/backend/catalog/heap.c

@@ -978,7 +978,7 @@ AddNewRelationType(const char *typeName,
 				   0,			/* array dimensions for typBaseType */
 				   false,		/* Type NOT NULL */
 				   InvalidOid,  /* rowtypes never have a collation */
-				   InvalidOid);
+				   InvalidOid);	/* typsubscription - none */
 }
 
 /* --------------------------------
@@ -1246,7 +1246,7 @@ heap_create_with_catalog(const char *relname,
 				   0,			/* array dimensions for typBaseType */
 				   false,		/* Type NOT NULL */
 				   InvalidOid,  /* rowtypes never have a collation */
-				   F_ARRAY_SUBSCRIPTION);
+				   F_ARRAY_SUBSCRIPTION);	/* array implementation */
 
 		pfree(relarrayname);
 	}

src/backend/commands/typecmds.c

@@ -642,7 +642,7 @@ DefineType(List *names, List *parameters)
 				   0,			/* Array Dimensions of typbasetype */
 				   false,		/* Type NOT NULL */
 				   collation,	/* type's collation */
-				   subscriptionOid);
+				   subscriptionOid);	/* subscription procedure */
 	Assert(typoid == address.objectId);
 
 	/*
@@ -1075,7 +1075,7 @@ DefineDomain(CreateDomainStmt *stmt)
 				   typNDims,	/* Array dimensions for base type */
 				   typNotNull,	/* Type NOT NULL */
 				   domaincoll,  /* type's collation */
-				   subscriptionProcedure);
+				   subscriptionProcedure);	/* subscription procedure */
 
 	/*
 	 * Process constraints which refer to the domain ID returned by TypeCreate
@@ -1188,7 +1188,7 @@ DefineEnum(CreateEnumStmt *stmt)
 				   0,			/* Array dimensions of typbasetype */
 				   false,		/* Type NOT NULL */
 				   InvalidOid,  /* type's collation */
-				   InvalidOid);
+				   InvalidOid);	/* typsubscription - none */
 
 	/* Enter the enum's values into pg_enum */
 	EnumValuesCreate(enumTypeAddr.objectId, stmt->vals);
@@ -1229,7 +1229,7 @@ DefineEnum(CreateEnumStmt *stmt)
 			   0,				/* Array dimensions of typbasetype */
 			   false,			/* Type NOT NULL */
 			   InvalidOid,		/* type's collation */
-			   F_ARRAY_SUBSCRIPTION);
+			   F_ARRAY_SUBSCRIPTION);	/* array subscription implementation */
 
 	pfree(enumArrayName);
 
@@ -1529,7 +1529,7 @@ DefineRange(CreateRangeStmt *stmt)
 				   0,			/* Array dimensions of typbasetype */
 				   false,		/* Type NOT NULL */
 				   InvalidOid,  /* type's collation (ranges never have one) */
-				   InvalidOid);
+				   InvalidOid);	/* typsubscription - none */
 	Assert(typoid == address.objectId);
 
 	/* Create the entry in pg_range */
@@ -1572,7 +1572,7 @@ DefineRange(CreateRangeStmt *stmt)
 			   0,				/* Array dimensions of typbasetype */
 			   false,			/* Type NOT NULL */
 			   InvalidOid,		/* typcollation */
-			   F_ARRAY_SUBSCRIPTION);
+			   F_ARRAY_SUBSCRIPTION);	/* array subscription implementation */
 
 	pfree(rangeArrayName);
 

src/backend/executor/execQual.c

@@ -253,9 +253,12 @@ static Datum ExecEvalGroupingFuncExpr(GroupingFuncExprState *gstate,
 /*----------
  *	  ExecEvalSubscriptionRef
  *
- *	   This function takes an SubscriptionRef and returns the extracted Datum
- *	   if it's a simple reference, or the modified containers value if it's
- *	   an containers assignment (i.e., containers element or slice insertion).
+ *	   This function takes a SubscriptionRef, extracts all information required
+ *	   for subscription and pass it to a particular subscription procedure,
+ *	   specified for this data type. As a result the extracted Datum will be
+ *	   returned if it's a simple reference, or the modified containers value if
+ *	   it's an containers assignment (i.e., containers element or slice
+ *	   insertion).
  *
  * NOTE: if we get a NULL result from a subscript expression, we return NULL
  * when it's an containers reference, or raise an error when it's an assignment.

src/backend/parser/parse_node.c

@@ -201,13 +201,18 @@ make_var(ParseState *pstate, RangeTblEntry *rte, int attrno, int location)
 
 /* NOTE: extra code for array type with isn't affect on other types
  * transformArrayType()
- *		Identify the types involved in a subscripting operation
+ *		Identify the types involved in a subscripting operation for array
  *
  * On entry, arrayType/arrayTypmod identify the type of the input value
  * to be subscripted (which could be a domain type).  These are modified
  * if necessary to identify the actual array type and typmod, and the
  * array's element type is returned.  An error is thrown if the input isn't
  * an array type.
+ *
+ * NOTE: This part of type-specific code is not separated into type-specific
+ * subscription procedure for now, but it does not affect on the whole logic,
+ * since InvalidOid will be return in case of other types not an error.
+ * An error will appears only if a subscription procedure is not defined.
  */
 Oid
 transformArrayType(Oid *containerType, int32 *containerTypmod)
@@ -257,29 +262,36 @@ transformArrayType(Oid *containerType, int32 *containerTypmod)
  *		Transform container subscripting.  This is used for both
  *		container fetch and container assignment.
  *
- * In a container fetch, we are given a source container value and we produce an
- * expression that represents the result of extracting a single container element
- * or a container slice.
+ * In a container fetch, we are given a source container value and we produce
+ * an expression that represents the result of extracting a single container
+ * element or a container slice.
  *
  * In a container assignment, we are given a destination container value plus a
- * source value that is to be assigned to a single element or a slice of
- * that container.  We produce an expression that represents the new container value
+ * source value that is to be assigned to a single element or a slice of that
+ * container.  We produce an expression that represents the new container value
  * with the source data inserted into the right part of the container.
  *
- * For both cases, if the source container is of a domain-over-container type,
- * the result is of the base container type or its element type; essentially,
- * we must fold a domain to its base type before applying subscripting.
- * (Note that int2vector and oidvector are treated as domains here.)
+ * For both cases, this function contains only general subscription logic while
+ * type-specific logic (e.g. type verifications and coersion) is placend in
+ * separate procedure indicated by typsubscription. There is only one exception
+ * for now about domain-over-container, if the source container is of a
+ * domain-over-container type, the result is of the base container type or its
+ * element type; essentially, we must fold a domain to its base type before
+ * applying subscripting.  (Note that int2vector and oidvector are treated as
+ * domains here.) If domain verification failed we assume, that element type
+ * must be the same as container type (e.g. in case of jsonb).
+ * An error will appear in case if current container type doesn't have a
+ * subscription procedure.
  *
- * pstate		Parse state
+ * pstate			Parse state
  * containerBase	Already-transformed expression for the container as a whole
  * containerType	OID of container's datatype (should match type of containerBase,
- *				or be the base type of containerBase's domain type)
- * elementType	OID of container's element type (fetch with transformcontainerType,
- *				or pass InvalidOid to do it here)
+ *					or be the base type of containerBase's domain type)
+ * elementType		OID of container's element type (fetch with transformArrayType,
+ *					or pass InvalidOid to do it here)
  * containerTypMod	typmod for the container (which is also typmod for the elements)
- * indirection	Untransformed list of subscripts (must not be NIL)
- * assignFrom	NULL for container fetch, else transformed expression for source.
+ * indirection		Untransformed list of subscripts (must not be NIL)
+ * assignFrom		NULL for container fetch, else transformed expression for source.
  */
 
 SubscriptionRef *

src/backend/parser/parse_target.c

@@ -776,7 +776,7 @@ transformAssignmentIndirection(ParseState *pstate,
 }
 
 /*
- * helper for transformAssignmentIndirection: process array assignment
+ * helper for transformAssignmentIndirection: process container assignment
  */
 static Node *
 transformAssignmentSubscripts(ParseState *pstate,

src/backend/utils/adt/arrayfuncs.c

@@ -6561,6 +6561,11 @@ isAssignmentIndirectionExpr(ExprState *exprstate)
 	return false;
 }
 
+/*
+ * Perform an actual data extraction or modification for the array
+ * subscription. As a result the extracted Datum or the modified containers
+ * value will be returned.
+ */
 Datum
 array_subscription_evaluate(PG_FUNCTION_ARGS)
 {
@@ -6722,6 +6727,13 @@ array_subscription_evaluate(PG_FUNCTION_ARGS)
 							   sbstate->refelemalign);
 }
 
+/*
+ * Perform preparation for the array subscription, mostly type verification
+ * and coersion. This function produces an expression that represents the
+ * result of extracting a single container element/container slice or the new
+ * container value with the source data inserted into the right part of the
+ * container.
+ */
 Datum
 array_subscription_prepare(PG_FUNCTION_ARGS)
 {
@@ -6860,6 +6872,9 @@ array_subscription_prepare(PG_FUNCTION_ARGS)
 	PG_RETURN_POINTER(sbsref);
 }
 
+/*
+ * Handle array-type subscription logic.
+ */
 Datum
 array_subscription(PG_FUNCTION_ARGS)
 {

src/backend/utils/adt/jsonb.c

@@ -1149,6 +1149,11 @@ to_jsonb(PG_FUNCTION_ARGS)
 	PG_RETURN_POINTER(JsonbValueToJsonb(res));
 }
 
+/*
+ * Do the actual conversion to jsonb for to_jsonb function. This logic is
+ * separated because it can be useful not only in here (e.g. we use it in
+ * jsonb subscription)
+ */
 JsonbValue *
 to_jsonb_worker(Datum source, Oid source_type)
 {

src/backend/utils/adt/jsonfuncs.c

@@ -1365,6 +1365,32 @@ jsonb_get_element(Jsonb *jb, Datum *path, int npath, bool *isnull, bool as_text)
 	}
 }
 
+Datum
+jsonb_set_element(Datum jsonbdatum, Datum *path, int path_len,
+				  Datum sourceData, Oid source_type)
+{
+	Jsonb			   *jb = DatumGetJsonb(jsonbdatum);
+	JsonbValue		   *newval,
+					   *res;
+	JsonbParseState    *state = NULL;
+	JsonbIterator 	   *it;
+	bool			   *path_nulls = palloc0(path_len * sizeof(bool));
+
+	newval = to_jsonb_worker(sourceData, source_type);
+
+	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(JsonbValueToJsonb(res));
+}
+
 /*
  * SQL function json_array_length(json) -> int
  */
@@ -4033,12 +4059,17 @@ 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)
 			{
-				(void) pushJsonbValue(st, WJB_ELEM, newval);
+				(void) pushJsonbValue(st, WJB_ELEM, new);
 			}
 		}
 	}
 }
 
+/*
+ * Perform an actual data extraction or modification for the jsonb
+ * subscription. As a result the extracted Datum or the modified containers
+ * value will be returned.
+ */
 Datum
 jsonb_subscription_evaluate(PG_FUNCTION_ARGS)
 {
@@ -4120,6 +4151,14 @@ jsonb_subscription_evaluate(PG_FUNCTION_ARGS)
 								 false);
 }
 
+/*
+ * Perform preparation for the jsonb subscription. Since there are not any
+ * particular restrictions for this kind of subscription, we will verify that
+ * it is not a slice operation. This function produces an expression that
+ * represents the result of extracting a single container element or the new
+ * container value with the source data inserted into the right part of the
+ * container.
+ */
 Datum
 jsonb_subscription_prepare(PG_FUNCTION_ARGS)
 {
@@ -4168,6 +4207,9 @@ jsonb_subscription_prepare(PG_FUNCTION_ARGS)
 	PG_RETURN_POINTER(sbsref);
 }
 
+/*
+ * Handle jsonb-type subscription logic.
+ */
 Datum
 jsonb_subscription(PG_FUNCTION_ARGS)
 {
@@ -4183,29 +4225,3 @@ jsonb_subscription(PG_FUNCTION_ARGS)
 
 	elog(ERROR, "incorrect op_type for subscription function: %d", op_type);
 }
-
-Datum
-jsonb_set_element(Datum jsonbdatum, Datum *path, int path_len,
-				  Datum sourceData, Oid source_type)
-{
-	Jsonb			   *jb = DatumGetJsonb(jsonbdatum);
-	JsonbValue		   *newval,
-					   *res;
-	JsonbParseState    *state = NULL;
-	JsonbIterator 	   *it;
-	bool			   *path_nulls = palloc0(path_len * sizeof(bool));
-
-	newval = to_jsonb_worker(sourceData, source_type);
-
-	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(JsonbValueToJsonb(res));
-}

src/include/catalog/pg_type.h

@@ -198,6 +198,11 @@ CATALOG(pg_type,1247) BKI_BOOTSTRAP BKI_ROWTYPE_OID(71) BKI_SCHEMA_MACRO
 	 * collatable base types, possibly other OID for domains
 	 */
 	Oid			typcollation;
+
+	/*
+	 * Type specific subscription logic. If typsubscription is none, it means
+	 * that this type doesn't support subscription.
+	 */
 	regproc		typsubscription;
 
 #ifdef CATALOG_VARLEN			/* variable-length fields start here */
@@ -224,7 +229,6 @@ CATALOG(pg_type,1247) BKI_BOOTSTRAP BKI_ROWTYPE_OID(71) BKI_SCHEMA_MACRO
 	 */
 	aclitem		typacl[1];
 #endif
-
 } FormData_pg_type;
 
 /* ----------------

src/include/nodes/execnodes.h

@@ -657,12 +657,21 @@ typedef struct SubscriptionRefExprState
 	List	   *reflowerindexpr;
 	ExprState  *refexpr;
 	ExprState  *refassgnexpr;
-	int16		refattrlength;	/* typlen of array type */
-	int16		refelemlength;	/* typlen of the array element type */
+	int16		refattrlength;	/* typlen of container type */
+	int16		refelemlength;	/* typlen of the container element type */
 	bool		refelembyval;	/* is the element type pass-by-value? */
 	char		refelemalign;	/* typalign of the element type */
 } SubscriptionRefExprState;
 
+/* ---------------------------------
+ * Subscription exec information
+ *
+ * It contains all information which is required to perform type-specific data
+ * extraction or modification. This information will be gathered in
+ * `ExecEvalSubscriptionRef` function and passed to `typsubscription`
+ * procedure.
+ * ---------------------------------
+ */
 typedef struct SubscriptionExecData
 {
 	ExprContext *xprcontext;			/* econtext for subscription */