On 06.01.26 17:44, Kirill Reshke wrote:
On Tue, 6 Jan 2026 at 21:28, Kirill Reshke <[email protected]> wrote:
On Mon, 5 Jan 2026 at 13:39, Andrey Borodin <[email protected]> wrote:
On 4 Jan 2026, at 00:25, Kirill Reshke <[email protected]> wrote:
PFA v6
Would it be theoretically possible to unite functions for different GIN page
types?
e.g. merge gin_entrypage_items + gin_datapage_items -> gin_tree_items? Or is it
an awkward API?
For this, I borrowed this design from HASH and BRIN pageinspect
implementation. For them, we have one function-per-page-type. So,
maybe we can have dynamic schema here, but I don't see this as an
improvement to design.
--
Best regards,
Kirill Reshke
CF bot did like trailing whitespace in regression output files,
posting v8 with this issue fixed.
I noticed that the newly added C code could use a bit of modernization.
See attached patch for some suggestions. I only treated the new
gin_entrypage_items(); the new gin_datapage_items() could use mostly the
same changes. One of the main changes is to push many local variables
to smaller scopes. Also some changes in ereport parentheses style,
palloc style, some whitespace changes. (The whole patch also needs
pgindent treatment.)
From b424062035a43c894cf6d11b854a917be0cad6d8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 7 Jan 2026 17:37:05 +0100
Subject: [PATCH] some code cleanup
---
contrib/pageinspect/ginfuncs.c | 95 ++++++++++++++++------------------
1 file changed, 44 insertions(+), 51 deletions(-)
diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c
index 3cd5e4c0e65..4294b640a9a 100644
--- a/contrib/pageinspect/ginfuncs.c
+++ b/contrib/pageinspect/ginfuncs.c
@@ -191,17 +191,16 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
Oid indexRelid = PG_GETARG_OID(1);
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
Relation indexRel;
- OffsetNumber maxoff, offset;
+ OffsetNumber maxoff;
TupleDesc tupdesc;
- bool oneCol;
Page page;
GinPageOpaque opaq;
StringInfoData buf;
if (!superuser())
ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser to use raw page
functions")));
+ errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to use raw page
functions"));
InitMaterializedSRF(fcinfo, 0);
@@ -210,9 +209,9 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
if (!IS_INDEX(indexRel) || !IS_GIN(indexRel))
ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a %s index",
-
RelationGetRelationName(indexRel), "GIN")));
+ errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a %s index",
+ RelationGetRelationName(indexRel),
"GIN"));
page = get_page_from_raw(raw_page);
@@ -224,58 +223,46 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GinPageOpaqueData)))
ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("input page is not a valid GIN
entry tree page"),
- errdetail("Expected special size %d,
got %d.",
- (int)
MAXALIGN(sizeof(GinPageOpaqueData)),
- (int)
PageGetSpecialSize(page))));
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("input page is not a valid GIN entry
tree page"),
+ errdetail("Expected special size %d, got %d.",
+ (int)
MAXALIGN(sizeof(GinPageOpaqueData)),
+ PageGetSpecialSize(page)));
opaq = GinPageGetOpaque(page);
-
/* we only support entry tree in this function, check that */
if (opaq->flags & GIN_META)
ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("gin_entrypage_items is
unsupported for metapage")));
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("gin_entrypage_items is unsupported for
metapage"));
if (opaq->flags & (GIN_LIST | GIN_LIST_FULLROW))
ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("gin_entrypage_items is
unsupported for fast list pages")));
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("gin_entrypage_items is unsupported for
fast list pages"));
if (opaq->flags & GIN_DATA)
ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("input page is not a GIN entry
tree page"),
- errhint("This appears to be a GIN
posting tree page. Please use gin_datapage_items")));
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("input page is not a GIN entry tree
page"),
+ errhint("This appears to be a GIN posting tree
page. Please use gin_datapage_items."));
initStringInfo(&buf);
maxoff = PageGetMaxOffsetNumber(page);
tupdesc = RelationGetDescr(indexRel);
- oneCol = tupdesc->natts == 1;
- for (offset = FirstOffsetNumber;
+ for (OffsetNumber offset = FirstOffsetNumber;
offset <= maxoff;
offset = OffsetNumberNext(offset))
{
OffsetNumber indAtt;
Datum values[4];
- bool nulls[4];
- int ndecoded, i;
- Datum *tids_datum;
- ItemPointer items_orig;
- bool free_items_orig;
+ bool nulls[4] = {0};
Datum attrVal;
- Oid foutoid;
- bool typisvarlena;
- Oid typoid;
- char* value;
- bool nq;
- char* tmp;
bool isnull;
IndexTuple idxtuple;
ItemId iid = PageGetItemId(page, offset);
@@ -285,20 +272,17 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
idxtuple = (IndexTuple) PageGetItem(page, iid);
- memset(nulls, 0, sizeof(nulls));
-
values[0] = UInt16GetDatum(offset);
- if (oneCol)
+ if (tupdesc->natts == 1)
{
indAtt = FirstOffsetNumber;
/* here we can safely reuse pg_class's tuple
descriptor. */
- attrVal = index_getattr(idxtuple, FirstOffsetNumber,
tupdesc,
- &isnull);
+ attrVal = index_getattr(idxtuple, FirstOffsetNumber,
tupdesc, &isnull);
if (isnull)
ereport(ERROR,
-
(errcode(ERRCODE_INDEX_CORRUPTED),
- errmsg("invalid gin
entry page tuple at offset %d", offset)));
+
errcode(ERRCODE_INDEX_CORRUPTED),
+ errmsg("invalid gin entry page
tuple at offset %d", offset));
}
else
{
@@ -313,14 +297,13 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
/* orig tuple reuse is safe */
- res = index_getattr(idxtuple, FirstOffsetNumber,
tupdesc,
- &isnull);
+ res = index_getattr(idxtuple, FirstOffsetNumber,
tupdesc, &isnull);
/* we do not expect null for first attr in multi-column
GIN */
if (isnull)
ereport(ERROR,
-
(errcode(ERRCODE_INDEX_CORRUPTED),
- errmsg("invalid gin
entry page tuple at offset %d", offset)));
+
errcode(ERRCODE_INDEX_CORRUPTED),
+ errmsg("invalid gin entry page
tuple at offset %d", offset));
indAtt = DatumGetUInt16(res);
@@ -346,16 +329,22 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
appendStringInfo(&buf, "%s=",
quote_identifier(TupleDescAttr(tupdesc, indAtt - 1)->attname.data));
- if (!isnull) {
+ if (!isnull)
+ {
+ Oid foutoid;
+ bool typisvarlena;
+ Oid typoid;
+ char* value;
+ bool nq;
+
/* Most of this is copied from record_out(). */
typoid = TupleDescAttr(tupdesc, indAtt - 1)->atttypid;
getTypeOutputInfo(typoid, &foutoid, &typisvarlena);
value = OidOutputFunctionCall(foutoid, attrVal);
-
/* Check whether we need double quotes for this value */
nq = (value[0] == '\0'); /* force quotes for
empty string */
- for (tmp = value; *tmp; tmp++)
+ for (const char *tmp = value; *tmp; tmp++)
{
char ch = *tmp;
@@ -371,7 +360,7 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
/* And emit the string */
if (nq)
appendStringInfoCharMacro(&buf, '"');
- for (tmp = value; *tmp; tmp++)
+ for (const char *tmp = value; *tmp; tmp++)
{
char ch = *tmp;
@@ -387,7 +376,6 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
appendStringInfo(&buf, "NULL");
}
-
values[3] = CStringGetTextDatum(buf.data);
resetStringInfo(&buf);
@@ -398,6 +386,11 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
}
else
{
+ int ndecoded;
+ Datum *tids_datum;
+ ItemPointer items_orig;
+ bool free_items_orig;
+
values[1] = ItemPointerGetDatum(&idxtuple->t_tid);
/* Get list of item pointers from the tuple. */
if (GinItupIsCompressed(idxtuple))
@@ -412,8 +405,8 @@ gin_entrypage_items(PG_FUNCTION_ARGS)
free_items_orig = false;
}
- tids_datum = (Datum *) palloc(ndecoded * sizeof(Datum));
- for (i = 0; i < ndecoded; i++)
+ tids_datum = palloc_array(Datum, ndecoded);
+ for (int i = 0; i < ndecoded; i++)
tids_datum[i] =
ItemPointerGetDatum(&items_orig[i]);
values[2] =
PointerGetDatum(construct_array_builtin(tids_datum, ndecoded, TIDOID));
--
2.52.0
