Redesign pageinspect function printing infomask bits

After more discussion, the new function added by ddbd5d8 could have been
designed in a better way.  Based on an idea from Álvaro, instead of
returning one column which includes both the raw and combined flags, use
two columns, with one for the raw flags and one for the combined flags.

This also takes care of some issues with HEAP_LOCKED_UPGRADED and
HEAP_XMAX_IS_LOCKED_ONLY which are not really combined flags as they
depend on conditions defined by other raw bits, as mentioned by Amit.

While on it, fix an extra issue with combined flags.  A combined flag
was returned if at least one of its bits was set, but all its bits need
to be set to include it in the result.

Author: Michael Paquier
Reviewed-by: Álvaro Herrera, Amit Kapila
Discussion: https://postgr.es/m/20190913114950.GA3824@alvherre.pgsql

Author: michaelpq

contrib/pageinspect/expected/page.out

@@ -86,127 +86,55 @@ SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
 -- always be the same in all test runs. we show raw flags by
 -- default: HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID.
 VACUUM FREEZE test1;
-SELECT t_infomask, t_infomask2, flags
+SELECT t_infomask, t_infomask2, raw_flags, combined_flags
 FROM heap_page_items(get_raw_page('test1', 0)),
-     LATERAL heap_tuple_infomask_flags(t_infomask, t_infomask2) m(flags);
- t_infomask | t_infomask2 |                           flags                           
-------------+-------------+-----------------------------------------------------------
-       2816 |           2 | {HEAP_XMAX_INVALID,HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID}
+     LATERAL heap_tuple_infomask_flags(t_infomask, t_infomask2);
+ t_infomask | t_infomask2 |                         raw_flags                         |   combined_flags   
+------------+-------------+-----------------------------------------------------------+--------------------
+       2816 |           2 | {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID} | {HEAP_XMIN_FROZEN}
 (1 row)
 
 -- output the decoded flag HEAP_XMIN_FROZEN instead
-SELECT t_infomask, t_infomask2, flags
+SELECT t_infomask, t_infomask2, raw_flags, combined_flags
 FROM heap_page_items(get_raw_page('test1', 0)),
-     LATERAL heap_tuple_infomask_flags(t_infomask, t_infomask2, true) m(flags);
- t_infomask | t_infomask2 |                flags                 
-------------+-------------+--------------------------------------
-       2816 |           2 | {HEAP_XMAX_INVALID,HEAP_XMIN_FROZEN}
+     LATERAL heap_tuple_infomask_flags(t_infomask, t_infomask2);
+ t_infomask | t_infomask2 |                         raw_flags                         |   combined_flags   
+------------+-------------+-----------------------------------------------------------+--------------------
+       2816 |           2 | {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID} | {HEAP_XMIN_FROZEN}
 (1 row)
 
 -- tests for decoding of combined flags
 -- HEAP_XMAX_SHR_LOCK = (HEAP_XMAX_EXCL_LOCK | HEAP_XMAX_KEYSHR_LOCK)
-SELECT heap_tuple_infomask_flags(x'0050'::int, 0, true);
- heap_tuple_infomask_flags 
----------------------------
- {HEAP_XMAX_SHR_LOCK}
-(1 row)
-
-SELECT heap_tuple_infomask_flags(x'0050'::int, 0, false);
-          heap_tuple_infomask_flags          
----------------------------------------------
- {HEAP_XMAX_EXCL_LOCK,HEAP_XMAX_KEYSHR_LOCK}
+SELECT * FROM heap_tuple_infomask_flags(x'0050'::int, 0);
+                  raw_flags                  |    combined_flags    
+---------------------------------------------+----------------------
+ {HEAP_XMAX_KEYSHR_LOCK,HEAP_XMAX_EXCL_LOCK} | {HEAP_XMAX_SHR_LOCK}
 (1 row)
 
 -- HEAP_XMIN_FROZEN = (HEAP_XMIN_COMMITTED | HEAP_XMIN_INVALID)
-SELECT heap_tuple_infomask_flags(x'0300'::int, 0, true);
- heap_tuple_infomask_flags 
----------------------------
- {HEAP_XMIN_FROZEN}
-(1 row)
-
-SELECT heap_tuple_infomask_flags(x'0300'::int, 0, false);
-        heap_tuple_infomask_flags        
------------------------------------------
- {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID}
+SELECT * FROM heap_tuple_infomask_flags(x'0300'::int, 0);
+                raw_flags                |   combined_flags   
+-----------------------------------------+--------------------
+ {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID} | {HEAP_XMIN_FROZEN}
 (1 row)
 
 -- HEAP_MOVED = (HEAP_MOVED_IN | HEAP_MOVED_OFF)
-SELECT heap_tuple_infomask_flags(x'C000'::int, 0, true);
- heap_tuple_infomask_flags 
----------------------------
- {HEAP_MOVED}
+SELECT * FROM heap_tuple_infomask_flags(x'C000'::int, 0);
+           raw_flags            | combined_flags 
+--------------------------------+----------------
+ {HEAP_MOVED_OFF,HEAP_MOVED_IN} | {HEAP_MOVED}
 (1 row)
 
-SELECT heap_tuple_infomask_flags(x'C000'::int, 0, false);
-   heap_tuple_infomask_flags    
---------------------------------
- {HEAP_MOVED_IN,HEAP_MOVED_OFF}
-(1 row)
-
--- HEAP_LOCKED_UPGRADED = (HEAP_XMAX_IS_MULTI | HEAP_XMAX_LOCK_ONLY)
-SELECT heap_tuple_infomask_flags(x'1080'::int, 0, true);
- heap_tuple_infomask_flags 
----------------------------
- {HEAP_LOCKED_UPGRADED}
-(1 row)
-
-SELECT heap_tuple_infomask_flags(x'1080'::int, 0, false);
-        heap_tuple_infomask_flags         
-------------------------------------------
- {HEAP_XMAX_LOCK_ONLY,HEAP_XMAX_IS_MULTI}
+SELECT * FROM heap_tuple_infomask_flags(x'C000'::int, 0);
+           raw_flags            | combined_flags 
+--------------------------------+----------------
+ {HEAP_MOVED_OFF,HEAP_MOVED_IN} | {HEAP_MOVED}
 (1 row)
 
 -- test all flags of t_infomask and t_infomask2
-SELECT unnest(heap_tuple_infomask_flags(x'FFFF'::int, x'FFFF'::int, false))
-  AS flags ORDER BY 1;
-         flags         
------------------------
- HEAP_COMBOCID
- HEAP_HASEXTERNAL
- HEAP_HASNULL
- HEAP_HASOID_OLD
- HEAP_HASVARWIDTH
- HEAP_HOT_UPDATED
- HEAP_KEYS_UPDATED
- HEAP_MOVED_IN
- HEAP_MOVED_OFF
- HEAP_ONLY_TUPLE
- HEAP_UPDATED
- HEAP_XMAX_COMMITTED
- HEAP_XMAX_EXCL_LOCK
- HEAP_XMAX_INVALID
- HEAP_XMAX_IS_MULTI
- HEAP_XMAX_KEYSHR_LOCK
- HEAP_XMAX_LOCK_ONLY
- HEAP_XMIN_COMMITTED
- HEAP_XMIN_INVALID
-(19 rows)
-
-SELECT unnest(heap_tuple_infomask_flags(x'FFFF'::int, x'FFFF'::int, true))
-  AS flags ORDER BY 1;
-        flags        
----------------------
- HEAP_COMBOCID
- HEAP_HASEXTERNAL
- HEAP_HASNULL
- HEAP_HASOID_OLD
- HEAP_HASVARWIDTH
- HEAP_HOT_UPDATED
- HEAP_KEYS_UPDATED
- HEAP_MOVED
- HEAP_ONLY_TUPLE
- HEAP_UPDATED
- HEAP_XMAX_COMMITTED
- HEAP_XMAX_INVALID
- HEAP_XMAX_IS_MULTI
- HEAP_XMAX_LOCK_ONLY
- HEAP_XMAX_SHR_LOCK
- HEAP_XMIN_FROZEN
-(16 rows)
-
-SELECT unnest(heap_tuple_infomask_flags(-1, -1, false))
-  AS flags ORDER BY 1;
-         flags         
+SELECT unnest(raw_flags)
+  FROM heap_tuple_infomask_flags(x'FFFF'::int, x'FFFF'::int) ORDER BY 1;
+        unnest         
 -----------------------
  HEAP_COMBOCID
  HEAP_HASEXTERNAL
@@ -229,38 +157,28 @@ SELECT unnest(heap_tuple_infomask_flags(-1, -1, false))
  HEAP_XMIN_INVALID
 (19 rows)
 
-SELECT unnest(heap_tuple_infomask_flags(-1, -1, true))
-  AS flags ORDER BY 1;
-        flags        
----------------------
- HEAP_COMBOCID
- HEAP_HASEXTERNAL
- HEAP_HASNULL
- HEAP_HASOID_OLD
- HEAP_HASVARWIDTH
- HEAP_HOT_UPDATED
- HEAP_KEYS_UPDATED
+SELECT unnest(combined_flags)
+  FROM heap_tuple_infomask_flags(x'FFFF'::int, x'FFFF'::int) ORDER BY 1;
+       unnest       
+--------------------
  HEAP_MOVED
- HEAP_ONLY_TUPLE
- HEAP_UPDATED
- HEAP_XMAX_COMMITTED
- HEAP_XMAX_INVALID
- HEAP_XMAX_IS_MULTI
- HEAP_XMAX_LOCK_ONLY
  HEAP_XMAX_SHR_LOCK
  HEAP_XMIN_FROZEN
-(16 rows)
+(3 rows)
 
--- no flags
-SELECT unnest(heap_tuple_infomask_flags(0, 0, false));
- unnest 
---------
-(0 rows)
+-- no flags at all
+SELECT * FROM heap_tuple_infomask_flags(0, 0);
+ raw_flags | combined_flags 
+-----------+----------------
+ {}        | {}
+(1 row)
 
-SELECT unnest(heap_tuple_infomask_flags(0, 0, true));
- unnest 
---------
-(0 rows)
+-- no combined flags
+SELECT * FROM heap_tuple_infomask_flags(x'0010'::int, 0);
+        raw_flags        | combined_flags 
+-------------------------+----------------
+ {HEAP_XMAX_KEYSHR_LOCK} | {}
+(1 row)
 
 DROP TABLE test1;
 -- check that using any of these functions with a partitioned table or index

contrib/pageinspect/heapfuncs.c

@@ -507,99 +507,117 @@ PG_FUNCTION_INFO_V1(heap_tuple_infomask_flags);
 Datum
 heap_tuple_infomask_flags(PG_FUNCTION_ARGS)
 {
+#define HEAP_TUPLE_INFOMASK_COLS 2
+	Datum		values[HEAP_TUPLE_INFOMASK_COLS];
+	bool		nulls[HEAP_TUPLE_INFOMASK_COLS];
 	uint16		t_infomask = PG_GETARG_INT16(0);
 	uint16		t_infomask2 = PG_GETARG_INT16(1);
-	bool		decode_combined = PG_GETARG_BOOL(2);
 	int			cnt = 0;
 	ArrayType  *a;
 	int			bitcnt;
-	Datum	   *d;
+	Datum	   *flags;
+	TupleDesc	tupdesc;
+	HeapTuple	tuple;
 
 	if (!superuser())
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("must be superuser to use raw page functions")));
 
+	/* Build a tuple descriptor for our result type */
+	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+		elog(ERROR, "return type must be a row type");
+
 	bitcnt = pg_popcount((const char *) &t_infomask, sizeof(uint16)) +
 		pg_popcount((const char *) &t_infomask2, sizeof(uint16));
 
-	/* If no flags, return an empty array */
+	/* Initialize values and NULL flags arrays */
+	MemSet(values, 0, sizeof(values));
+	MemSet(nulls, 0, sizeof(nulls));
+
+	/* If no flags, return a set of empty arrays */
 	if (bitcnt <= 0)
-		PG_RETURN_POINTER(construct_empty_array(TEXTOID));
+	{
+		values[0] = PointerGetDatum(construct_empty_array(TEXTOID));
+		values[1] = PointerGetDatum(construct_empty_array(TEXTOID));
+		tuple = heap_form_tuple(tupdesc, values, nulls);
+		PG_RETURN_DATUM(HeapTupleGetDatum(tuple));
+	}
 
-	d = (Datum *) palloc0(sizeof(Datum) * bitcnt);
+	/* build set of raw flags */
+	flags = (Datum *) palloc0(sizeof(Datum) * bitcnt);
 
 	/* decode t_infomask */
 	if ((t_infomask & HEAP_HASNULL) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_HASNULL");
+		flags[cnt++] = CStringGetTextDatum("HEAP_HASNULL");
 	if ((t_infomask & HEAP_HASVARWIDTH) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_HASVARWIDTH");
+		flags[cnt++] = CStringGetTextDatum("HEAP_HASVARWIDTH");
 	if ((t_infomask & HEAP_HASEXTERNAL) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_HASEXTERNAL");
+		flags[cnt++] = CStringGetTextDatum("HEAP_HASEXTERNAL");
 	if ((t_infomask & HEAP_HASOID_OLD) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_HASOID_OLD");
+		flags[cnt++] = CStringGetTextDatum("HEAP_HASOID_OLD");
+	if ((t_infomask & HEAP_XMAX_KEYSHR_LOCK) != 0)
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMAX_KEYSHR_LOCK");
 	if ((t_infomask & HEAP_COMBOCID) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_COMBOCID");
+		flags[cnt++] = CStringGetTextDatum("HEAP_COMBOCID");
+	if ((t_infomask & HEAP_XMAX_EXCL_LOCK) != 0)
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMAX_EXCL_LOCK");
+	if ((t_infomask & HEAP_XMAX_LOCK_ONLY) != 0)
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMAX_LOCK_ONLY");
+	if ((t_infomask & HEAP_XMIN_COMMITTED) != 0)
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMIN_COMMITTED");
+	if ((t_infomask & HEAP_XMIN_INVALID) != 0)
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMIN_INVALID");
 	if ((t_infomask & HEAP_XMAX_COMMITTED) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_XMAX_COMMITTED");
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMAX_COMMITTED");
 	if ((t_infomask & HEAP_XMAX_INVALID) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_XMAX_INVALID");
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMAX_INVALID");
+	if ((t_infomask & HEAP_XMAX_IS_MULTI) != 0)
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMAX_IS_MULTI");
 	if ((t_infomask & HEAP_UPDATED) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_UPDATED");
-
-	/* decode combined masks of t_infomaks */
-	if (decode_combined && (t_infomask & HEAP_XMAX_SHR_LOCK) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_XMAX_SHR_LOCK");
-	else
-	{
-		if ((t_infomask & HEAP_XMAX_EXCL_LOCK) != 0)
-			d[cnt++] = CStringGetTextDatum("HEAP_XMAX_EXCL_LOCK");
-		if ((t_infomask & HEAP_XMAX_KEYSHR_LOCK) != 0)
-			d[cnt++] = CStringGetTextDatum("HEAP_XMAX_KEYSHR_LOCK");
-	}
-
-	if (decode_combined && (t_infomask & HEAP_XMIN_FROZEN) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_XMIN_FROZEN");
-	else
-	{
-		if ((t_infomask & HEAP_XMIN_COMMITTED) != 0)
-			d[cnt++] = CStringGetTextDatum("HEAP_XMIN_COMMITTED");
-		if ((t_infomask & HEAP_XMIN_INVALID) != 0)
-			d[cnt++] = CStringGetTextDatum("HEAP_XMIN_INVALID");
-	}
-
-	if (decode_combined && (t_infomask & HEAP_MOVED) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_MOVED");
-	else
-	{
-		if ((t_infomask & HEAP_MOVED_IN) != 0)
-			d[cnt++] = CStringGetTextDatum("HEAP_MOVED_IN");
-		if ((t_infomask & HEAP_MOVED_OFF) != 0)
-			d[cnt++] = CStringGetTextDatum("HEAP_MOVED_OFF");
-	}
-
-	if (decode_combined && HEAP_LOCKED_UPGRADED(t_infomask))
-		d[cnt++] = CStringGetTextDatum("HEAP_LOCKED_UPGRADED");
-	else
-	{
-		if (HEAP_XMAX_IS_LOCKED_ONLY(t_infomask))
-			d[cnt++] = CStringGetTextDatum("HEAP_XMAX_LOCK_ONLY");
-		if ((t_infomask & HEAP_XMAX_IS_MULTI) != 0)
-			d[cnt++] = CStringGetTextDatum("HEAP_XMAX_IS_MULTI");
-	}
+		flags[cnt++] = CStringGetTextDatum("HEAP_UPDATED");
+	if ((t_infomask & HEAP_MOVED_OFF) != 0)
+		flags[cnt++] = CStringGetTextDatum("HEAP_MOVED_OFF");
+	if ((t_infomask & HEAP_MOVED_IN) != 0)
+		flags[cnt++] = CStringGetTextDatum("HEAP_MOVED_IN");
 
 	/* decode t_infomask2 */
 	if ((t_infomask2 & HEAP_KEYS_UPDATED) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_KEYS_UPDATED");
+		flags[cnt++] = CStringGetTextDatum("HEAP_KEYS_UPDATED");
 	if ((t_infomask2 & HEAP_HOT_UPDATED) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_HOT_UPDATED");
+		flags[cnt++] = CStringGetTextDatum("HEAP_HOT_UPDATED");
 	if ((t_infomask2 & HEAP_ONLY_TUPLE) != 0)
-		d[cnt++] = CStringGetTextDatum("HEAP_ONLY_TUPLE");
+		flags[cnt++] = CStringGetTextDatum("HEAP_ONLY_TUPLE");
 
+	/* build value */
 	Assert(cnt <= bitcnt);
-	a = construct_array(d, cnt, TEXTOID, -1, false, 'i');
+	a = construct_array(flags, cnt, TEXTOID, -1, false, 'i');
+	values[0] = PointerGetDatum(a);
 
-	pfree(d);
+	/*
+	 * Build set of combined flags.  Use the same array as previously, this
+	 * keeps the code simple.
+	 */
+	cnt = 0;
+	MemSet(flags, 0, sizeof(Datum) * bitcnt);
+
+	/* decode combined masks of t_infomask */
+	if ((t_infomask & HEAP_XMAX_SHR_LOCK) == HEAP_XMAX_SHR_LOCK)
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMAX_SHR_LOCK");
+	if ((t_infomask & HEAP_XMIN_FROZEN) == HEAP_XMIN_FROZEN)
+		flags[cnt++] = CStringGetTextDatum("HEAP_XMIN_FROZEN");
+	if ((t_infomask & HEAP_MOVED) == HEAP_MOVED)
+		flags[cnt++] = CStringGetTextDatum("HEAP_MOVED");
+
+	/* Build an empty array if there are no combined flags */
+	if (cnt == 0)
+		a = construct_empty_array(TEXTOID);
+	else
+		a = construct_array(flags, cnt, TEXTOID, -1, false, 'i');
+	pfree(flags);
+	values[1] = PointerGetDatum(a);
 
-	PG_RETURN_POINTER(a);
+	/* Returns the record as Datum */
+	tuple = heap_form_tuple(tupdesc, values, nulls);
+	PG_RETURN_DATUM(HeapTupleGetDatum(tuple));
 }