Fix varatt versus Datum type confusions

Macros like VARDATA() and VARSIZE() should be thought of as taking
values of type pointer to struct varlena or some other related struct.
The way they are implemented, you can pass anything to it and it will
cast it right.  But this is in principle incorrect.  To fix, add the
required DatumGetPointer() calls.  Or in a couple of cases, remove
superfluous PointerGetDatum() calls.

It is planned in a subsequent patch to change macros like VARDATA()
and VARSIZE() to inline functions, which will enforce stricter typing.
This is in preparation for that.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/928ea48f-77c6-417b-897c-621ef16685a6%40eisentraut.org

contrib/hstore/hstore_gin.c

@@ -127,7 +127,7 @@ gin_extract_hstore_query(PG_FUNCTION_ARGS)
 			/* Nulls in the array are ignored, cf hstoreArrayToPairs */
 			if (key_nulls[i])
 				continue;
-			item = makeitem(VARDATA(key_datums[i]), VARSIZE(key_datums[i]) - VARHDRSZ, KEYFLAG);
+			item = makeitem(VARDATA(DatumGetPointer(key_datums[i])), VARSIZE(DatumGetPointer(key_datums[i])) - VARHDRSZ, KEYFLAG);
 			entries[j++] = PointerGetDatum(item);
 		}
 

contrib/hstore/hstore_gist.c

@@ -576,7 +576,7 @@ ghstore_consistent(PG_FUNCTION_ARGS)
 
 			if (key_nulls[i])
 				continue;
-			crc = crc32_sz(VARDATA(key_datums[i]), VARSIZE(key_datums[i]) - VARHDRSZ);
+			crc = crc32_sz(VARDATA(DatumGetPointer(key_datums[i])), VARSIZE(DatumGetPointer(key_datums[i])) - VARHDRSZ);
 			if (!(GETBIT(sign, HASHVAL(crc, siglen))))
 				res = false;
 		}
@@ -599,7 +599,7 @@ ghstore_consistent(PG_FUNCTION_ARGS)
 
 			if (key_nulls[i])
 				continue;
-			crc = crc32_sz(VARDATA(key_datums[i]), VARSIZE(key_datums[i]) - VARHDRSZ);
+			crc = crc32_sz(VARDATA(DatumGetPointer(key_datums[i])), VARSIZE(DatumGetPointer(key_datums[i])) - VARHDRSZ);
 			if (GETBIT(sign, HASHVAL(crc, siglen)))
 				res = true;
 		}

contrib/hstore/hstore_io.c

@@ -684,22 +684,22 @@ hstore_from_arrays(PG_FUNCTION_ARGS)
 
 		if (!value_nulls || value_nulls[i])
 		{
-			pairs[i].key = VARDATA(key_datums[i]);
+			pairs[i].key = VARDATA(DatumGetPointer(key_datums[i]));
 			pairs[i].val = NULL;
 			pairs[i].keylen =
-				hstoreCheckKeyLen(VARSIZE(key_datums[i]) - VARHDRSZ);
+				hstoreCheckKeyLen(VARSIZE(DatumGetPointer(key_datums[i])) - VARHDRSZ);
 			pairs[i].vallen = 4;
 			pairs[i].isnull = true;
 			pairs[i].needfree = false;
 		}
 		else
 		{
-			pairs[i].key = VARDATA(key_datums[i]);
+			pairs[i].key = VARDATA(DatumGetPointer(key_datums[i]));
-			pairs[i].val = VARDATA(value_datums[i]);
+			pairs[i].val = VARDATA(DatumGetPointer(value_datums[i]));
 			pairs[i].keylen =
-				hstoreCheckKeyLen(VARSIZE(key_datums[i]) - VARHDRSZ);
+				hstoreCheckKeyLen(VARSIZE(DatumGetPointer(key_datums[i])) - VARHDRSZ);
 			pairs[i].vallen =
-				hstoreCheckValLen(VARSIZE(value_datums[i]) - VARHDRSZ);
+				hstoreCheckValLen(VARSIZE(DatumGetPointer(value_datums[i])) - VARHDRSZ);
 			pairs[i].isnull = false;
 			pairs[i].needfree = false;
 		}
@@ -778,22 +778,22 @@ hstore_from_array(PG_FUNCTION_ARGS)
 
 		if (in_nulls[i * 2 + 1])
 		{
-			pairs[i].key = VARDATA(in_datums[i * 2]);
+			pairs[i].key = VARDATA(DatumGetPointer(in_datums[i * 2]));
 			pairs[i].val = NULL;
 			pairs[i].keylen =
-				hstoreCheckKeyLen(VARSIZE(in_datums[i * 2]) - VARHDRSZ);
+				hstoreCheckKeyLen(VARSIZE(DatumGetPointer(in_datums[i * 2])) - VARHDRSZ);
 			pairs[i].vallen = 4;
 			pairs[i].isnull = true;
 			pairs[i].needfree = false;
 		}
 		else
 		{
-			pairs[i].key = VARDATA(in_datums[i * 2]);
+			pairs[i].key = VARDATA(DatumGetPointer(in_datums[i * 2]));
-			pairs[i].val = VARDATA(in_datums[i * 2 + 1]);
+			pairs[i].val = VARDATA(DatumGetPointer(in_datums[i * 2 + 1]));
 			pairs[i].keylen =
-				hstoreCheckKeyLen(VARSIZE(in_datums[i * 2]) - VARHDRSZ);
+				hstoreCheckKeyLen(VARSIZE(DatumGetPointer(in_datums[i * 2])) - VARHDRSZ);
 			pairs[i].vallen =
-				hstoreCheckValLen(VARSIZE(in_datums[i * 2 + 1]) - VARHDRSZ);
+				hstoreCheckValLen(VARSIZE(DatumGetPointer(in_datums[i * 2 + 1])) - VARHDRSZ);
 			pairs[i].isnull = false;
 			pairs[i].needfree = false;
 		}

contrib/hstore/hstore_op.c

@@ -107,8 +107,8 @@ hstoreArrayToPairs(ArrayType *a, int *npairs)
 	{
 		if (!key_nulls[i])
 		{
-			key_pairs[j].key = VARDATA(key_datums[i]);
+			key_pairs[j].key = VARDATA(DatumGetPointer(key_datums[i]));
-			key_pairs[j].keylen = VARSIZE(key_datums[i]) - VARHDRSZ;
+			key_pairs[j].keylen = VARSIZE(DatumGetPointer(key_datums[i])) - VARHDRSZ;
 			key_pairs[j].val = NULL;
 			key_pairs[j].vallen = 0;
 			key_pairs[j].needfree = 0;

contrib/test_decoding/test_decoding.c

@@ -581,7 +581,7 @@ tuple_to_stringinfo(StringInfo s, TupleDesc tupdesc, HeapTuple tuple, bool skip_
 		/* print data */
 		if (isnull)
 			appendStringInfoString(s, "null");
-		else if (typisvarlena && VARATT_IS_EXTERNAL_ONDISK(origval))
+		else if (typisvarlena && VARATT_IS_EXTERNAL_ONDISK(DatumGetPointer(origval)))
 			appendStringInfoString(s, "unchanged-toast-datum");
 		else if (!typisvarlena)
 			print_literal(s, typid,

src/backend/access/brin/brin_minmax_multi.c

@@ -624,7 +624,7 @@ brin_range_serialize(Ranges *range)
 
 		for (i = 0; i < nvalues; i++)
 		{
-			len += VARSIZE_ANY(range->values[i]);
+			len += VARSIZE_ANY(DatumGetPointer(range->values[i]));
 		}
 	}
 	else if (typlen == -2)		/* cstring */

src/backend/access/common/heaptuple.c

@@ -189,7 +189,7 @@ getmissingattr(TupleDesc tupleDesc,
 			if (att->attlen > 0)
 				key.len = att->attlen;
 			else
-				key.len = VARSIZE_ANY(attrmiss->am_value);
+				key.len = VARSIZE_ANY(DatumGetPointer(attrmiss->am_value));
 			key.value = attrmiss->am_value;
 
 			entry = hash_search(missing_cache, &key, HASH_ENTER, &found);
@@ -901,9 +901,9 @@ expand_tuple(HeapTuple *targetHeapTuple,
 												  att->attlen,
 												  attrmiss[attnum].am_value);
 
-				targetDataLen = att_addlength_pointer(targetDataLen,
+				targetDataLen = att_addlength_datum(targetDataLen,
-													  att->attlen,
+													att->attlen,
-													  attrmiss[attnum].am_value);
+													attrmiss[attnum].am_value);
 			}
 			else
 			{

src/backend/access/common/reloptions.c

@@ -1190,8 +1190,8 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
 
 		for (i = 0; i < noldoptions; i++)
 		{
-			char	   *text_str = VARDATA(oldoptions[i]);
+			char	   *text_str = VARDATA(DatumGetPointer(oldoptions[i]));
-			int			text_len = VARSIZE(oldoptions[i]) - VARHDRSZ;
+			int			text_len = VARSIZE(DatumGetPointer(oldoptions[i])) - VARHDRSZ;
 
 			/* Search for a match in defList */
 			foreach(cell, defList)
@@ -1456,8 +1456,8 @@ parseRelOptionsInternal(Datum options, bool validate,
 
 	for (i = 0; i < noptions; i++)
 	{
-		char	   *text_str = VARDATA(optiondatums[i]);
+		char	   *text_str = VARDATA(DatumGetPointer(optiondatums[i]));
-		int			text_len = VARSIZE(optiondatums[i]) - VARHDRSZ;
+		int			text_len = VARSIZE(DatumGetPointer(optiondatums[i])) - VARHDRSZ;
 		int			j;
 
 		/* Search for a match in reloptions */

src/backend/access/common/toast_internals.c

@@ -144,7 +144,7 @@ toast_save_datum(Relation rel, Datum value,
 	int			num_indexes;
 	int			validIndex;
 
-	Assert(!VARATT_IS_EXTERNAL(value));
+	Assert(!VARATT_IS_EXTERNAL(dval));
 
 	/*
 	 * Open the toast relation and its indexes.  We can use the index to check

src/backend/access/gin/gininsert.c

@@ -2233,7 +2233,7 @@ _gin_build_tuple(OffsetNumber attrnum, unsigned char category,
 	else if (typlen > 0)
 		keylen = typlen;
 	else if (typlen == -1)
-		keylen = VARSIZE_ANY(key);
+		keylen = VARSIZE_ANY(DatumGetPointer(key));
 	else if (typlen == -2)
 		keylen = strlen(DatumGetPointer(key)) + 1;
 	else

src/backend/access/spgist/spgutils.c

@@ -785,7 +785,7 @@ SpGistGetInnerTypeSize(SpGistTypeDesc *att, Datum datum)
 	else if (att->attlen > 0)
 		size = att->attlen;
 	else
-		size = VARSIZE_ANY(datum);
+		size = VARSIZE_ANY(DatumGetPointer(datum));
 
 	return MAXALIGN(size);
 }
@@ -804,7 +804,7 @@ memcpyInnerDatum(void *target, SpGistTypeDesc *att, Datum datum)
 	}
 	else
 	{
-		size = (att->attlen > 0) ? att->attlen : VARSIZE_ANY(datum);
+		size = (att->attlen > 0) ? att->attlen : VARSIZE_ANY(DatumGetPointer(datum));
 		memcpy(target, DatumGetPointer(datum), size);
 	}
 }

src/backend/access/table/toast_helper.c

@@ -330,7 +330,7 @@ toast_delete_external(Relation rel, const Datum *values, const bool *isnull,
 
 			if (isnull[i])
 				continue;
-			else if (VARATT_IS_EXTERNAL_ONDISK(value))
+			else if (VARATT_IS_EXTERNAL_ONDISK(DatumGetPointer(value)))
 				toast_delete_datum(rel, value, is_speculative);
 		}
 	}

src/backend/replication/logical/proto.c

@@ -809,7 +809,7 @@ logicalrep_write_tuple(StringInfo out, Relation rel, TupleTableSlot *slot,
 			continue;
 		}
 
-		if (att->attlen == -1 && VARATT_IS_EXTERNAL_ONDISK(values[i]))
+		if (att->attlen == -1 && VARATT_IS_EXTERNAL_ONDISK(DatumGetPointer(values[i])))
 		{
 			/*
 			 * Unchanged toasted datum.  (Note that we don't promise to detect

src/backend/replication/pgoutput/pgoutput.c

@@ -1374,8 +1374,8 @@ pgoutput_row_filter(Relation relation, TupleTableSlot *old_slot,
 		 * VARTAG_INDIRECT. See ReorderBufferToastReplace.
 		 */
 		if (att->attlen == -1 &&
-			VARATT_IS_EXTERNAL_ONDISK(new_slot->tts_values[i]) &&
+			VARATT_IS_EXTERNAL_ONDISK(DatumGetPointer(new_slot->tts_values[i])) &&
-			!VARATT_IS_EXTERNAL_ONDISK(old_slot->tts_values[i]))
+			!VARATT_IS_EXTERNAL_ONDISK(DatumGetPointer(old_slot->tts_values[i])))
 		{
 			if (!tmp_new_slot)
 			{

src/backend/statistics/mcv.c

@@ -767,7 +767,7 @@ statext_mcv_serialize(MCVList *mcvlist, VacAttrStats **stats)
 				values[dim][i] = PointerGetDatum(PG_DETOAST_DATUM(values[dim][i]));
 
 				/* serialized length (uint32 length + data) */
-				len = VARSIZE_ANY_EXHDR(values[dim][i]);
+				len = VARSIZE_ANY_EXHDR(DatumGetPointer(values[dim][i]));
 				info[dim].nbytes += sizeof(uint32); /* length */
 				info[dim].nbytes += len;	/* value (no header) */
 

src/backend/tsearch/ts_selfuncs.c

@@ -233,7 +233,7 @@ mcelem_tsquery_selec(TSQuery query, Datum *mcelem, int nmcelem,
 		 * The text Datums came from an array, so it cannot be compressed or
 		 * stored out-of-line -- it's safe to use VARSIZE_ANY*.
 		 */
-		Assert(!VARATT_IS_COMPRESSED(mcelem[i]) && !VARATT_IS_EXTERNAL(mcelem[i]));
+		Assert(!VARATT_IS_COMPRESSED(DatumGetPointer(mcelem[i])) && !VARATT_IS_EXTERNAL(DatumGetPointer(mcelem[i])));
 		lookup[i].element = (text *) DatumGetPointer(mcelem[i]);
 		lookup[i].frequency = numbers[i];
 	}

src/backend/utils/adt/jsonb_gin.c

@@ -896,8 +896,8 @@ gin_extract_jsonb_query(PG_FUNCTION_ARGS)
 				continue;
 			/* We rely on the array elements not being toasted */
 			entries[j++] = make_text_key(JGINFLAG_KEY,
-										 VARDATA_ANY(key_datums[i]),
+										 VARDATA_ANY(DatumGetPointer(key_datums[i])),
-										 VARSIZE_ANY_EXHDR(key_datums[i]));
+										 VARSIZE_ANY_EXHDR(DatumGetPointer(key_datums[i])));
 		}
 
 		*nentries = j;

src/backend/utils/adt/jsonb_op.c

@@ -63,8 +63,8 @@ jsonb_exists_any(PG_FUNCTION_ARGS)
 
 		strVal.type = jbvString;
 		/* We rely on the array elements not being toasted */
-		strVal.val.string.val = VARDATA_ANY(key_datums[i]);
+		strVal.val.string.val = VARDATA_ANY(DatumGetPointer(key_datums[i]));
-		strVal.val.string.len = VARSIZE_ANY_EXHDR(key_datums[i]);
+		strVal.val.string.len = VARSIZE_ANY_EXHDR(DatumGetPointer(key_datums[i]));
 
 		if (findJsonbValueFromContainer(&jb->root,
 										JB_FOBJECT | JB_FARRAY,
@@ -96,8 +96,8 @@ jsonb_exists_all(PG_FUNCTION_ARGS)
 
 		strVal.type = jbvString;
 		/* We rely on the array elements not being toasted */
-		strVal.val.string.val = VARDATA_ANY(key_datums[i]);
+		strVal.val.string.val = VARDATA_ANY(DatumGetPointer(key_datums[i]));
-		strVal.val.string.len = VARSIZE_ANY_EXHDR(key_datums[i]);
+		strVal.val.string.len = VARSIZE_ANY_EXHDR(DatumGetPointer(key_datums[i]));
 
 		if (findJsonbValueFromContainer(&jb->root,
 										JB_FOBJECT | JB_FARRAY,

src/backend/utils/adt/jsonfuncs.c

@@ -4766,8 +4766,8 @@ jsonb_delete_array(PG_FUNCTION_ARGS)
 					continue;
 
 				/* We rely on the array elements not being toasted */
-				keyptr = VARDATA_ANY(keys_elems[i]);
+				keyptr = VARDATA_ANY(DatumGetPointer(keys_elems[i]));
-				keylen = VARSIZE_ANY_EXHDR(keys_elems[i]);
+				keylen = VARSIZE_ANY_EXHDR(DatumGetPointer(keys_elems[i]));
 				if (keylen == v.val.string.len &&
 					memcmp(keyptr, v.val.string.val, keylen) == 0)
 				{

src/backend/utils/adt/jsonpath_exec.c

@@ -3074,8 +3074,8 @@ JsonItemFromDatum(Datum val, Oid typid, int32 typmod, JsonbValue *res)
 		case TEXTOID:
 		case VARCHAROID:
 			res->type = jbvString;
-			res->val.string.val = VARDATA_ANY(val);
+			res->val.string.val = VARDATA_ANY(DatumGetPointer(val));
-			res->val.string.len = VARSIZE_ANY_EXHDR(val);
+			res->val.string.len = VARSIZE_ANY_EXHDR(DatumGetPointer(val));
 			break;
 		case DATEOID:
 		case TIMEOID:

src/backend/utils/adt/multirangetypes.c

@@ -394,12 +394,13 @@ multirange_send(PG_FUNCTION_ARGS)
 	for (int i = 0; i < range_count; i++)
 	{
 		Datum		range;
+		bytea	   *outputbytes;
 
 		range = RangeTypePGetDatum(ranges[i]);
-		range = PointerGetDatum(SendFunctionCall(&cache->typioproc, range));
+		outputbytes = SendFunctionCall(&cache->typioproc, range);
 
-		pq_sendint32(buf, VARSIZE(range) - VARHDRSZ);
+		pq_sendint32(buf, VARSIZE(outputbytes) - VARHDRSZ);
-		pq_sendbytes(buf, VARDATA(range), VARSIZE(range) - VARHDRSZ);
+		pq_sendbytes(buf, VARDATA(outputbytes), VARSIZE(outputbytes) - VARHDRSZ);
 	}
 
 	PG_RETURN_BYTEA_P(pq_endtypsend(buf));

src/backend/utils/adt/rangetypes.c

@@ -285,8 +285,7 @@ range_send(PG_FUNCTION_ARGS)
 
 	if (RANGE_HAS_LBOUND(flags))
 	{
-		Datum		bound = PointerGetDatum(SendFunctionCall(&cache->typioproc,
+		bytea	   *bound = SendFunctionCall(&cache->typioproc, lower.val);
-															 lower.val));
 		uint32		bound_len = VARSIZE(bound) - VARHDRSZ;
 		char	   *bound_data = VARDATA(bound);
 
@@ -296,8 +295,7 @@ range_send(PG_FUNCTION_ARGS)
 
 	if (RANGE_HAS_UBOUND(flags))
 	{
-		Datum		bound = PointerGetDatum(SendFunctionCall(&cache->typioproc,
+		bytea	   *bound = SendFunctionCall(&cache->typioproc, upper.val);
-															 upper.val));
 		uint32		bound_len = VARSIZE(bound) - VARHDRSZ;
 		char	   *bound_data = VARDATA(bound);
 

src/backend/utils/adt/tsvector_op.c

@@ -329,8 +329,8 @@ tsvector_setweight_by_filter(PG_FUNCTION_ARGS)
 		if (nulls[i])
 			continue;
 
-		lex = VARDATA(dlexemes[i]);
+		lex = VARDATA(DatumGetPointer(dlexemes[i]));
-		lex_len = VARSIZE(dlexemes[i]) - VARHDRSZ;
+		lex_len = VARSIZE(DatumGetPointer(dlexemes[i])) - VARHDRSZ;
 		lex_pos = tsvector_bsearch(tsout, lex, lex_len);
 
 		if (lex_pos >= 0 && (j = POSDATALEN(tsout, entry + lex_pos)) != 0)
@@ -443,10 +443,10 @@ compare_text_lexemes(const void *va, const void *vb)
 {
 	Datum		a = *((const Datum *) va);
 	Datum		b = *((const Datum *) vb);
-	char	   *alex = VARDATA_ANY(a);
+	char	   *alex = VARDATA_ANY(DatumGetPointer(a));
-	int			alex_len = VARSIZE_ANY_EXHDR(a);
+	int			alex_len = VARSIZE_ANY_EXHDR(DatumGetPointer(a));
-	char	   *blex = VARDATA_ANY(b);
+	char	   *blex = VARDATA_ANY(DatumGetPointer(b));
-	int			blex_len = VARSIZE_ANY_EXHDR(b);
+	int			blex_len = VARSIZE_ANY_EXHDR(DatumGetPointer(b));
 
 	return tsCompareString(alex, alex_len, blex, blex_len, false);
 }
@@ -605,8 +605,8 @@ tsvector_delete_arr(PG_FUNCTION_ARGS)
 		if (nulls[i])
 			continue;
 
-		lex = VARDATA(dlexemes[i]);
+		lex = VARDATA(DatumGetPointer(dlexemes[i]));
-		lex_len = VARSIZE(dlexemes[i]) - VARHDRSZ;
+		lex_len = VARSIZE(DatumGetPointer(dlexemes[i])) - VARHDRSZ;
 		lex_pos = tsvector_bsearch(tsin, lex, lex_len);
 
 		if (lex_pos >= 0)
@@ -770,7 +770,7 @@ array_to_tsvector(PG_FUNCTION_ARGS)
 					(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
 					 errmsg("lexeme array may not contain nulls")));
 
-		if (VARSIZE(dlexemes[i]) - VARHDRSZ == 0)
+		if (VARSIZE(DatumGetPointer(dlexemes[i])) - VARHDRSZ == 0)
 			ereport(ERROR,
 					(errcode(ERRCODE_ZERO_LENGTH_CHARACTER_STRING),
 					 errmsg("lexeme array may not contain empty strings")));
@@ -786,7 +786,7 @@ array_to_tsvector(PG_FUNCTION_ARGS)
 
 	/* Calculate space needed for surviving lexemes. */
 	for (i = 0; i < nitems; i++)
-		datalen += VARSIZE(dlexemes[i]) - VARHDRSZ;
+		datalen += VARSIZE(DatumGetPointer(dlexemes[i])) - VARHDRSZ;
 	tslen = CALCDATASIZE(nitems, datalen);
 
 	/* Allocate and fill tsvector. */
@@ -798,8 +798,8 @@ array_to_tsvector(PG_FUNCTION_ARGS)
 	cur = STRPTR(tsout);
 	for (i = 0; i < nitems; i++)
 	{
-		char	   *lex = VARDATA(dlexemes[i]);
+		char	   *lex = VARDATA(DatumGetPointer(dlexemes[i]));
-		int			lex_len = VARSIZE(dlexemes[i]) - VARHDRSZ;
+		int			lex_len = VARSIZE(DatumGetPointer(dlexemes[i])) - VARHDRSZ;
 
 		memcpy(cur, lex, lex_len);
 		arrout[i].haspos = 0;