Hi,
As I am not seeing any response from Tomas for last 2-3 days and since
the commit-fest is coming towards end, I have planned to work on the
review comments that I had given few days back and submit the updated
patch. PFA new version of patch that takes care of all the review
comments given by me. I have also ran pgindent on btreefuncs.c file
and have run some basic test cases. All looks fine to me now!
Please note that this patch still belongs to Tomas not me. I still
remain the reviewer of this patch. The same thing has been very
clearly mentioned in the attached patch. The only intention behind
Ashutosh (reviewer) working on this patch is to ensure that we do not
miss the things that can easily get committed in this commit-fest.
Thanks.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
On Tue, Apr 4, 2017 at 7:23 PM, David Steele <da...@pgmasters.net> wrote:
> On 4/4/17 9:43 AM, Robert Haas wrote:
>> On Tue, Apr 4, 2017 at 9:32 AM, David Steele <da...@pgmasters.net> wrote:
>>> My goal is to help people focus on patches that have a chance. At this
>>> point I think that includes poking authors who are not being responsive
>>> using the limited means at my disposal.
>>
>> +1. Pings on specific threads can help clear things up when, for
>> example, the author and reviewer are each waiting for the other. And,
>> as you say, they also help avoid the situation where a patch just
>> falls off the radar and misses the release for no especially good
>> reason, which naturally causes people frustration.
>>
>> I think your pings have been quite helpful, although I think it would
>> have been better in some cases if you'd done them sooner. Pinging
>> after a week, with a 3 day deadline, when there are only a few days
>> left in the CommitFest isn't really leaving a lot of room to maneuver.
>
> Thanks for the feedback! My thinking is that I don't want to bug people
> too soon, but there's a maximum number of days a thread should be idle.
> Over the course of the CF that has gone from 10 days, to 7, 5, and 3.
>
> I don't look at all patches every day so it can be a bit uneven, i.e.,
> all patches are allowed certain amount of idle time, but some may get a
> bit more depending on when I check up on them.
>
> Thanks,
> --
> -David
> da...@pgmasters.net
From 6d9814182eb03521f093d687eb8024c9df1c11d5 Mon Sep 17 00:00:00 2001
From: ashu <ashu@localhost.localdomain>
Date: Tue, 4 Apr 2017 21:38:42 +0530
Subject: [PATCH] pageinspect: Add bt_page_items function with bytea v6
Author: Tomas Vondra <tomas.von...@2ndquadrant.com>
Reviewed-by: Ashutosh Sharma <ashu.coe...@gmail.com>
---
contrib/pageinspect/btreefuncs.c | 198 +++++++++++++++++++-------
contrib/pageinspect/expected/btree.out | 13 ++
contrib/pageinspect/pageinspect--1.5--1.6.sql | 14 ++
contrib/pageinspect/sql/btree.sql | 4 +
doc/src/sgml/pageinspect.sgml | 31 ++++
src/include/access/nbtree.h | 1 +
6 files changed, 210 insertions(+), 51 deletions(-)
diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 3d69aa9..02440ec 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -41,6 +41,7 @@
PG_FUNCTION_INFO_V1(bt_metap);
PG_FUNCTION_INFO_V1(bt_page_items);
+PG_FUNCTION_INFO_V1(bt_page_items_bytea);
PG_FUNCTION_INFO_V1(bt_page_stats);
#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
@@ -235,14 +236,6 @@ bt_page_stats(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(result);
}
-/*-------------------------------------------------------
- * bt_page_items()
- *
- * Get IndexTupleData set in a btree page
- *
- * Usage: SELECT * FROM bt_page_items('t1_pkey', 1);
- *-------------------------------------------------------
- */
/*
* cross-call data structure for SRF
@@ -253,14 +246,72 @@ struct user_args
OffsetNumber offset;
};
+/*-------------------------------------------------------
+ * bt_page_print_tuples()
+ *
+ * Form a tuple describing index tuple at a given offset
+ * ------------------------------------------------------
+ */
+static Datum
+bt_page_print_tuples(FuncCallContext *fctx, Page page, OffsetNumber offset)
+{
+ char *values[6];
+ HeapTuple tuple;
+ ItemId id;
+ IndexTuple itup;
+ int j;
+ int off;
+ int dlen;
+ char *dump;
+ char *ptr;
+
+ id = PageGetItemId(page, offset);
+
+ if (!ItemIdIsValid(id))
+ elog(ERROR, "invalid ItemId");
+
+ itup = (IndexTuple) PageGetItem(page, id);
+
+ j = 0;
+ values[j++] = psprintf("%d", offset);
+ values[j++] = psprintf("(%u,%u)",
+ ItemPointerGetBlockNumberNoCheck(&itup->t_tid),
+ ItemPointerGetOffsetNumberNoCheck(&itup->t_tid));
+ values[j++] = psprintf("%d", (int) IndexTupleSize(itup));
+ values[j++] = psprintf("%c", IndexTupleHasNulls(itup) ? 't' : 'f');
+ values[j++] = psprintf("%c", IndexTupleHasVarwidths(itup) ? 't' : 'f');
+
+ ptr = (char *) itup + IndexInfoFindDataOffset(itup->t_info);
+ dlen = IndexTupleSize(itup) - IndexInfoFindDataOffset(itup->t_info);
+ dump = palloc0(dlen * 3 + 1);
+ values[j] = dump;
+ for (off = 0; off < dlen; off++)
+ {
+ if (off > 0)
+ *dump++ = ' ';
+ sprintf(dump, "%02x", *(ptr + off) & 0xff);
+ dump += 2;
+ }
+
+ tuple = BuildTupleFromCStrings(fctx->attinmeta, values);
+
+ return HeapTupleGetDatum(tuple);
+}
+
+/*-------------------------------------------------------
+ * bt_page_items()
+ *
+ * Get IndexTupleData set in a btree page
+ *
+ * Usage: SELECT * FROM bt_page_items('t1_pkey', 1);
+ *-------------------------------------------------------
+ */
Datum
bt_page_items(PG_FUNCTION_ARGS)
{
text *relname = PG_GETARG_TEXT_PP(0);
uint32 blkno = PG_GETARG_UINT32(1);
Datum result;
- char *values[6];
- HeapTuple tuple;
FuncCallContext *fctx;
MemoryContext mctx;
struct user_args *uargs;
@@ -345,47 +396,8 @@ bt_page_items(PG_FUNCTION_ARGS)
if (fctx->call_cntr < fctx->max_calls)
{
- ItemId id;
- IndexTuple itup;
- int j;
- int off;
- int dlen;
- char *dump;
- char *ptr;
-
- id = PageGetItemId(uargs->page, uargs->offset);
-
- if (!ItemIdIsValid(id))
- elog(ERROR, "invalid ItemId");
-
- itup = (IndexTuple) PageGetItem(uargs->page, id);
-
- j = 0;
- values[j++] = psprintf("%d", uargs->offset);
- values[j++] = psprintf("(%u,%u)",
- ItemPointerGetBlockNumberNoCheck(&itup->t_tid),
- ItemPointerGetOffsetNumberNoCheck(&itup->t_tid));
- values[j++] = psprintf("%d", (int) IndexTupleSize(itup));
- values[j++] = psprintf("%c", IndexTupleHasNulls(itup) ? 't' : 'f');
- values[j++] = psprintf("%c", IndexTupleHasVarwidths(itup) ? 't' : 'f');
-
- ptr = (char *) itup + IndexInfoFindDataOffset(itup->t_info);
- dlen = IndexTupleSize(itup) - IndexInfoFindDataOffset(itup->t_info);
- dump = palloc0(dlen * 3 + 1);
- values[j] = dump;
- for (off = 0; off < dlen; off++)
- {
- if (off > 0)
- *dump++ = ' ';
- sprintf(dump, "%02x", *(ptr + off) & 0xff);
- dump += 2;
- }
-
- tuple = BuildTupleFromCStrings(fctx->attinmeta, values);
- result = HeapTupleGetDatum(tuple);
-
- uargs->offset = uargs->offset + 1;
-
+ result = bt_page_print_tuples(fctx, uargs->page, uargs->offset);
+ uargs->offset++;
SRF_RETURN_NEXT(fctx, result);
}
else
@@ -396,6 +408,90 @@ bt_page_items(PG_FUNCTION_ARGS)
}
}
+/*-------------------------------------------------------
+ * bt_page_items_bytea()
+ *
+ * Get IndexTupleData set in a btree page
+ *
+ * Usage: SELECT * FROM bt_page_items(get_raw_page('t1_pkey', 1));
+ *-------------------------------------------------------
+ */
+
+Datum
+bt_page_items_bytea(PG_FUNCTION_ARGS)
+{
+ bytea *raw_page = PG_GETARG_BYTEA_P(0);
+ Datum result;
+ FuncCallContext *fctx;
+ struct user_args *uargs;
+ int raw_page_size;
+
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be superuser to use pageinspect functions"))));
+
+ if (SRF_IS_FIRSTCALL())
+ {
+ BTPageOpaque opaque;
+ MemoryContext mctx;
+ TupleDesc tupleDesc;
+
+ raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
+
+ if (raw_page_size < SizeOfPageHeaderData)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("input page too small (%d bytes)", raw_page_size)));
+
+ fctx = SRF_FIRSTCALL_INIT();
+ mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
+
+ uargs = palloc(sizeof(struct user_args));
+
+ uargs->page = VARDATA(raw_page);
+
+ uargs->offset = FirstOffsetNumber;
+
+ opaque = (BTPageOpaque) PageGetSpecialPointer(uargs->page);
+
+ if (P_ISMETA(opaque))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("block is a meta page")));
+
+ if (P_ISDELETED(opaque))
+ elog(NOTICE, "page is deleted");
+
+ fctx->max_calls = PageGetMaxOffsetNumber(uargs->page);
+
+ /* Build a tuple descriptor for our result type */
+ if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+
+ fctx->attinmeta = TupleDescGetAttInMetadata(tupleDesc);
+
+ fctx->user_fctx = uargs;
+
+ MemoryContextSwitchTo(mctx);
+ }
+
+ fctx = SRF_PERCALL_SETUP();
+ uargs = fctx->user_fctx;
+
+ if (fctx->call_cntr < fctx->max_calls)
+ {
+ result = bt_page_print_tuples(fctx, uargs->page, uargs->offset);
+ uargs->offset++;
+ SRF_RETURN_NEXT(fctx, result);
+ }
+ else
+ {
+ pfree(uargs);
+ SRF_RETURN_DONE(fctx);
+ }
+}
+
/* ------------------------------------------------
* bt_metap()
diff --git a/contrib/pageinspect/expected/btree.out b/contrib/pageinspect/expected/btree.out
index 82a49e3..67b103a 100644
--- a/contrib/pageinspect/expected/btree.out
+++ b/contrib/pageinspect/expected/btree.out
@@ -42,4 +42,17 @@ data | 01 00 00 00 00 00 00 01
SELECT * FROM bt_page_items('test1_a_idx', 2);
ERROR: block number out of range
+SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 0));
+ERROR: block is a meta page
+SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 1));
+-[ RECORD 1 ]-----------------------
+itemoffset | 1
+ctid | (0,1)
+itemlen | 16
+nulls | f
+vars | f
+data | 01 00 00 00 00 00 00 01
+
+SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 2));
+ERROR: block number 2 is out of range for relation "test1_a_idx"
DROP TABLE test1;
diff --git a/contrib/pageinspect/pageinspect--1.5--1.6.sql b/contrib/pageinspect/pageinspect--1.5--1.6.sql
index 6ac2a80..c435628 100644
--- a/contrib/pageinspect/pageinspect--1.5--1.6.sql
+++ b/contrib/pageinspect/pageinspect--1.5--1.6.sql
@@ -83,3 +83,17 @@ CREATE FUNCTION page_checksum(IN page bytea, IN blkno int4)
RETURNS smallint
AS 'MODULE_PATHNAME', 'page_checksum'
LANGUAGE C STRICT PARALLEL SAFE;
+
+--
+-- bt_page_items_bytea()
+--
+CREATE FUNCTION bt_page_items(IN page bytea,
+ OUT itemoffset smallint,
+ OUT ctid tid,
+ OUT itemlen smallint,
+ OUT nulls bool,
+ OUT vars bool,
+ OUT data text)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'bt_page_items_bytea'
+LANGUAGE C STRICT PARALLEL SAFE;
diff --git a/contrib/pageinspect/sql/btree.sql b/contrib/pageinspect/sql/btree.sql
index 1eafc32..8eac64c 100644
--- a/contrib/pageinspect/sql/btree.sql
+++ b/contrib/pageinspect/sql/btree.sql
@@ -14,4 +14,8 @@ SELECT * FROM bt_page_items('test1_a_idx', 0);
SELECT * FROM bt_page_items('test1_a_idx', 1);
SELECT * FROM bt_page_items('test1_a_idx', 2);
+SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 0));
+SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 1));
+SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 2));
+
DROP TABLE test1;
diff --git a/doc/src/sgml/pageinspect.sgml b/doc/src/sgml/pageinspect.sgml
index c87b160..499d3e9 100644
--- a/doc/src/sgml/pageinspect.sgml
+++ b/doc/src/sgml/pageinspect.sgml
@@ -333,6 +333,37 @@ test=# SELECT * FROM bt_page_items('pg_cast_oid_index', 1);
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term>
+ <function>bt_page_items(page bytea) returns setof record</function>
+ <indexterm>
+ <primary>bt_page_items</primary>
+ </indexterm>
+ </term>
+
+ <listitem>
+ <para>
+ Similarly to <function>heap_page_items</function>, it is also possible to
+ pass the page to <function>bt_page_items</function> as a <type>bytea</>
+ value. So the last example may also be rewritten like this:
+<screen>
+test=# SELECT * FROM bt_page_items(get_raw_page('pg_cast_oid_index', 1));
+ itemoffset | ctid | itemlen | nulls | vars | data
+------------+---------+---------+-------+------+-------------
+ 1 | (0,1) | 12 | f | f | 23 27 00 00
+ 2 | (0,2) | 12 | f | f | 24 27 00 00
+ 3 | (0,3) | 12 | f | f | 25 27 00 00
+ 4 | (0,4) | 12 | f | f | 26 27 00 00
+ 5 | (0,5) | 12 | f | f | 27 27 00 00
+ 6 | (0,6) | 12 | f | f | 28 27 00 00
+ 7 | (0,7) | 12 | f | f | 29 27 00 00
+ 8 | (0,8) | 12 | f | f | 2a 27 00 00
+</screen>
+ All the other details are the same as explained in the previous section.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</sect2>
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index f9304db..15771ce 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -176,6 +176,7 @@ typedef struct BTMetaPageData
#define P_ISLEAF(opaque) ((opaque)->btpo_flags & BTP_LEAF)
#define P_ISROOT(opaque) ((opaque)->btpo_flags & BTP_ROOT)
#define P_ISDELETED(opaque) ((opaque)->btpo_flags & BTP_DELETED)
+#define P_ISMETA(opaque) ((opaque)->btpo_flags & BTP_META)
#define P_ISHALFDEAD(opaque) ((opaque)->btpo_flags & BTP_HALF_DEAD)
#define P_IGNORE(opaque) ((opaque)->btpo_flags & (BTP_DELETED|BTP_HALF_DEAD))
#define P_HAS_GARBAGE(opaque) ((opaque)->btpo_flags & BTP_HAS_GARBAGE)
--
1.8.3.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
