On Thu, 1 Jan 2026 at 19:00, Andrey Borodin <[email protected]> wrote: > > > > On 29 Dec 2025, at 17:51, Kirill Reshke <[email protected]> wrote: > > > > Attached v2 > > I've looked into the patch. > The functionality is useful and seems to reflect pageinspect style.
Thank you for taking a look.
> > Patch still is very raw, many things to improve.
>
> Yup, but let's work on it!
>
> Please update the documentation here [0].
>
> Other AM's seems to defend from each other: if (!IS_INDEX(rel) ||
> !IS_BTREE(rel)) or if (!IS_GIST(indexRel)). I don't see such check in new
> functions. B-tree also protects from temp tables of other sessions: if
> (RELATION_IS_OTHER_TEMP(rel)).
I have added protections for functions that accept regclass.
RELATION_IS_OTHER_TEMP is unnecessary here, because generic
get_raw_page functions already take care of that.
> gin_datapage_items() seem to ignore reloid, did you have some ideas how to
> use it?
Yes, I removed that 'reloid'. It was of no use.
> In gin_entrypage_items() buf and tmpTupdesc seem to be recreated for every
> offset, can we reuse them?
Well, I did not get it, how we can reuse them, but v3 now releases
memory after use.
For tmpTupdesc, the issue here is that GIN entry tree pages can have
tuples which need different tuple descriptors to access data, on the
same page. That's why I re-evaluate on every offset. I think caching
tuple descriptors here would be a cure worse than a disease.
Example from my first email here:
```
reshke=# select * from gin_entrypage_items(get_raw_page('x_i_j_idx',
1), 'x_i_j_idx'::regclass);
itemoffset | downlink | tids | keys
------------+----------+------+------------------------------------
1 | (3,0) | {} | i=113
2 | (5,0) | {} | j=34173cb38f07f89ddbebc2ac9128303f
3 | (2,0) | {} | j=a0a080f42e6f13b3a2df133f073095dd
4 | (4,0) | {} | j=fc490ca45c00b1249bbe3554a4fdf6fb
(4 rows)
```
To display that, we need different tupdescs for offset 1 & 2. for
offsets 3&4 we can reuse tupdesc from offset 2, but this would
(unnecessary?) increase patch complexity.
WDYT?
> gin_entrypage_items() errors out with "input page is not a valid GIN data
> leaf page", but function is for entry pages.
I have added more sanity checks and regression tests for them.
> There's no tests for gin_datapage_items().
I have added them in v3. My main concern here was that we have to
generate many tuples to trigger GIN to create an internal posting tree
page,
It is also block-size-dependent, and will not work for values other
than 8192, actually. Maybe we need to compute the number of inserted
tuples in-flight?
> There's a typo "unsuppoerted" and "rejrect" in gin.sql.
Fxd
> gin_entrypage_items() emits elog(NOTICE, "page is deleted"), but
> gin_datapage_items() does not.
This is stupid copy-paste from GiST pageinspect. My fault, removed.
(GiST deleted page do not store info to compute max offset correctly,
while GIN does store maxOffset in its Opaque info)
> Also, note that corresponding GiST code does "OffsetNumber maxoff =
> InvalidOffsetNumber;", but gin_entrypage_items() has no this initialization.
It actually does but in a separate line, is this a problem?
> I'd convert Assert(!isnull) into if+elog: inspected data might be corrupted.
I do not have a strong opinion here, but we are implementing
pageinspect here, not amcheck? I can only see one ereport with
corruption error code in pageinspect, and it is for HASH indexes.
Anyway, I changed assert to if-elog
> Thanks!
>
>
> Best regards, Andrey Borodin.
>
>
> [0]
> https://www.postgresql.org/docs/current/pageinspect.html#PAGEINSPECT-GIN-FUNCS
PFA v3. I added you and Romka (cc-ed) as reviewers. Romka did a review
of v1 off-list which triggered me to actually bump this thread.
--
Best regards,
Kirill Reshke
v3-0001-GIN-pageinspect-support-for-entry-tree-and-postin.patch
Description: Binary data
