On Fri, Sep 13, 2019 at 09:59:40AM +0530, Amit Kapila wrote: > I think that is what we have not done in one of the cases pointed by me.
Thinking more about it, I see your point now. HEAP_LOCKED_UPGRADED is not a direct combination of the other flags and depends on other conditions, so we cannot make a combination of it with other things. The three others don't have that problem. Attached is a patch to fix your suggestions. This also removes the use of HEAP_XMAX_IS_LOCKED_ONLY which did not make completely sense either as a "raw" flag. While on it, the order of the flags can be improved to match more the order of htup_details.h Does this patch address your concerns? -- Michael
diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out
index 6a09d46a57..76f02dbea2 100644
--- a/contrib/pageinspect/expected/page.out
+++ b/contrib/pageinspect/expected/page.out
@@ -91,7 +91,7 @@ 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}
+ 2816 | 2 | {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID}
(1 row)
-- output the decoded flag HEAP_XMIN_FROZEN instead
@@ -100,7 +100,7 @@ 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}
+ 2816 | 2 | {HEAP_XMIN_FROZEN,HEAP_XMAX_INVALID}
(1 row)
-- tests for decoding of combined flags
@@ -140,20 +140,7 @@ SELECT heap_tuple_infomask_flags(x'C000'::int, 0, true);
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}
+ {HEAP_MOVED_OFF,HEAP_MOVED_IN}
(1 row)
-- test all flags of t_infomask and t_infomask2
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 68f16cd400..c696d7d6d1 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -540,12 +540,8 @@ heap_tuple_infomask_flags(PG_FUNCTION_ARGS)
d[cnt++] = CStringGetTextDatum("HEAP_HASOID_OLD");
if ((t_infomask & HEAP_COMBOCID) != 0)
d[cnt++] = CStringGetTextDatum("HEAP_COMBOCID");
- if ((t_infomask & HEAP_XMAX_COMMITTED) != 0)
- d[cnt++] = CStringGetTextDatum("HEAP_XMAX_COMMITTED");
- if ((t_infomask & HEAP_XMAX_INVALID) != 0)
- d[cnt++] = CStringGetTextDatum("HEAP_XMAX_INVALID");
- if ((t_infomask & HEAP_UPDATED) != 0)
- d[cnt++] = CStringGetTextDatum("HEAP_UPDATED");
+ if ((t_infomask & HEAP_XMAX_LOCK_ONLY) != 0)
+ d[cnt++] = CStringGetTextDatum("HEAP_XMAX_LOCK_ONLY");
/* decode combined masks of t_infomaks */
if (decode_combined && (t_infomask & HEAP_XMAX_SHR_LOCK) != 0)
@@ -568,24 +564,23 @@ heap_tuple_infomask_flags(PG_FUNCTION_ARGS)
d[cnt++] = CStringGetTextDatum("HEAP_XMIN_INVALID");
}
+ if ((t_infomask & HEAP_XMAX_COMMITTED) != 0)
+ d[cnt++] = CStringGetTextDatum("HEAP_XMAX_COMMITTED");
+ if ((t_infomask & HEAP_XMAX_INVALID) != 0)
+ d[cnt++] = CStringGetTextDatum("HEAP_XMAX_INVALID");
+ if ((t_infomask & HEAP_XMAX_IS_MULTI) != 0)
+ d[cnt++] = CStringGetTextDatum("HEAP_XMAX_IS_MULTI");
+ if ((t_infomask & HEAP_UPDATED) != 0)
+ d[cnt++] = CStringGetTextDatum("HEAP_UPDATED");
+
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");
+ if ((t_infomask & HEAP_MOVED_IN) != 0)
+ d[cnt++] = CStringGetTextDatum("HEAP_MOVED_IN");
}
/* decode t_infomask2 */
diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql
index 0319b5fa11..c04351dba9 100644
--- a/contrib/pageinspect/sql/page.sql
+++ b/contrib/pageinspect/sql/page.sql
@@ -55,9 +55,6 @@ SELECT heap_tuple_infomask_flags(x'0300'::int, 0, false);
-- HEAP_MOVED = (HEAP_MOVED_IN | HEAP_MOVED_OFF)
SELECT heap_tuple_infomask_flags(x'C000'::int, 0, true);
SELECT heap_tuple_infomask_flags(x'C000'::int, 0, false);
--- HEAP_LOCKED_UPGRADED = (HEAP_XMAX_IS_MULTI | HEAP_XMAX_LOCK_ONLY)
-SELECT heap_tuple_infomask_flags(x'1080'::int, 0, true);
-SELECT heap_tuple_infomask_flags(x'1080'::int, 0, false);
-- test all flags of t_infomask and t_infomask2
SELECT unnest(heap_tuple_infomask_flags(x'FFFF'::int, x'FFFF'::int, false))
signature.asc
Description: PGP signature
