Change array_offset to return subscripts, not offsets

... and rename it and its sibling array_offsets to array_position and
array_positions, to account for the changed behavior.

Having the functions return subscripts better matches existing practice,
and is better suited to using the result value as a subscript into the
array directly.  For one-based arrays, the new definition is identical
to what was originally committed.

(We use the term "subscript" in the documentation, which is what we use
whenever we talk about arrays; but the functions themselves are named
using the word "position" to match the standard-defined POSITION()
functions.)

Author: Pavel Stěhule
Behavioral problem noted by Dean Rasheed.

1 files changed, 29 insertions, 25 deletions

diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
index 57074e0f46e..c0bfd33174c 100644
--- a/src/backend/utils/adt/array_userfuncs.c
+++ b/src/backend/utils/adt/array_userfuncs.c
@@ -19,7 +19,7 @@
 #include "utils/typcache.h"
 
 
-static Datum array_offset_common(FunctionCallInfo fcinfo);
+static Datum array_position_common(FunctionCallInfo fcinfo);
 
 
 /*
@@ -659,7 +659,7 @@ array_agg_array_finalfn(PG_FUNCTION_ARGS)
 }
 
 /*-----------------------------------------------------------------------------
- * array_offset, array_offset_start :
+ * array_position, array_position_start :
  *			return the offset of a value in an array.
  *
  * IS NOT DISTINCT FROM semantics are used for comparisons.  Return NULL when
@@ -667,26 +667,26 @@ array_agg_array_finalfn(PG_FUNCTION_ARGS)
  *-----------------------------------------------------------------------------
  */
 Datum
-array_offset(PG_FUNCTION_ARGS)
+array_position(PG_FUNCTION_ARGS)
 {
-	return array_offset_common(fcinfo);
+	return array_position_common(fcinfo);
 }
 
 Datum
-array_offset_start(PG_FUNCTION_ARGS)
+array_position_start(PG_FUNCTION_ARGS)
 {
-	return array_offset_common(fcinfo);
+	return array_position_common(fcinfo);
 }
 
 /*
- * array_offset_common
- * 		Common code for array_offset and array_offset_start
+ * array_position_common
+ * 		Common code for array_position and array_position_start
  *
  * These are separate wrappers for the sake of opr_sanity regression test.
  * They are not strict so we have to test for null inputs explicitly.
  */
 static Datum
-array_offset_common(FunctionCallInfo fcinfo)
+array_position_common(FunctionCallInfo fcinfo)
 {
 	ArrayType  *array;
 	Oid			collation = PG_GET_COLLATION();
@@ -694,8 +694,8 @@ array_offset_common(FunctionCallInfo fcinfo)
 	Datum		searched_element,
 				value;
 	bool		isnull;
-	int			offset = 0,
-				offset_min;
+	int			position,
+				position_min;
 	bool		found = false;
 	TypeCacheEntry *typentry;
 	ArrayMetaState *my_extra;
@@ -731,18 +731,20 @@ array_offset_common(FunctionCallInfo fcinfo)
 		null_search = false;
 	}
 
+	position = (ARR_LBOUND(array))[0] - 1;
+
 	/* figure out where to start */
 	if (PG_NARGS() == 3)
 	{
 		if (PG_ARGISNULL(2))
 			ereport(ERROR,
 					(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-					 errmsg("initial offset should not be NULL")));
+					 errmsg("initial position should not be NULL")));
 
-		offset_min = PG_GETARG_INT32(2);
+		position_min = PG_GETARG_INT32(2);
 	}
 	else
-		offset_min = 1;
+		position_min = (ARR_LBOUND(array))[0];
 
 	/*
 	 * We arrange to look up type info for array_create_iterator only once per
@@ -780,10 +782,10 @@ array_offset_common(FunctionCallInfo fcinfo)
 	array_iterator = array_create_iterator(array, 0, my_extra);
 	while (array_iterate(array_iterator, &value, &isnull))
 	{
-		offset += 1;
+		position++;
 
 		/* skip initial elements if caller requested so */
-		if (offset < offset_min)
+		if (position < position_min)
 			continue;
 
 		/*
@@ -818,12 +820,12 @@ array_offset_common(FunctionCallInfo fcinfo)
 	if (!found)
 		PG_RETURN_NULL();
 
-	PG_RETURN_INT32(offset);
+	PG_RETURN_INT32(position);
 }
 
 /*-----------------------------------------------------------------------------
- * array_offsets :
- *			return an array of offsets of a value in an array.
+ * array_positions :
+ *			return an array of positions of a value in an array.
  *
  * IS NOT DISTINCT FROM semantics are used for comparisons.  Returns NULL when
  * the input array is NULL.  When the value is not found in the array, returns
@@ -833,7 +835,7 @@ array_offset_common(FunctionCallInfo fcinfo)
  *-----------------------------------------------------------------------------
  */
 Datum
-array_offsets(PG_FUNCTION_ARGS)
+array_positions(PG_FUNCTION_ARGS)
 {
 	ArrayType  *array;
 	Oid			collation = PG_GET_COLLATION();
@@ -841,7 +843,7 @@ array_offsets(PG_FUNCTION_ARGS)
 	Datum		searched_element,
 				value;
 	bool		isnull;
-	int			offset = 0;
+	int			position;
 	TypeCacheEntry *typentry;
 	ArrayMetaState *my_extra;
 	bool		null_search;
@@ -854,6 +856,8 @@ array_offsets(PG_FUNCTION_ARGS)
 	array = PG_GETARG_ARRAYTYPE_P(0);
 	element_type = ARR_ELEMTYPE(array);
 
+	position = (ARR_LBOUND(array))[0] - 1;
+
 	/*
 	 * We refuse to search for elements in multi-dimensional arrays, since we
 	 * have no good way to report the element's location in the array.
@@ -912,12 +916,12 @@ array_offsets(PG_FUNCTION_ARGS)
 	}
 
 	/*
-	 * Accumulate each array offset iff the element matches the given element.
+	 * Accumulate each array position iff the element matches the given element.
 	 */
 	array_iterator = array_create_iterator(array, 0, my_extra);
 	while (array_iterate(array_iterator, &value, &isnull))
 	{
-		offset += 1;
+		position += 1;
 
 		/*
 		 * Can't look at the array element's value if it's null; but if we
@@ -927,7 +931,7 @@ array_offsets(PG_FUNCTION_ARGS)
 		{
 			if (isnull && null_search)
 				astate =
-					accumArrayResult(astate, Int32GetDatum(offset), false,
+					accumArrayResult(astate, Int32GetDatum(position), false,
 									 INT4OID, CurrentMemoryContext);
 
 			continue;
@@ -937,7 +941,7 @@ array_offsets(PG_FUNCTION_ARGS)
 		if (DatumGetBool(FunctionCall2Coll(&my_extra->proc, collation,
 										   searched_element, value)))
 			astate =
-				accumArrayResult(astate, Int32GetDatum(offset), false,
+				accumArrayResult(astate, Int32GetDatum(position), false,
 								 INT4OID, CurrentMemoryContext);
 	}