| From: | Kirill Reshke <reshkekirill(at)gmail(dot)com> |
|---|---|
| To: | Japin Li <japinli(at)hotmail(dot)com> |
| Cc: | Roman Khapov <rkhapov(at)yandex-team(dot)ru>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: GIN pageinspect support for entry tree and posting tree |
| Date: | 2026-01-14 12:40:52 |
| Message-ID: | CALdSSPgpT6omwswahChNoEuE1P+zO8nSZ377RTdPSrud_hwJmw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 14 Jan 2026 at 17:17, Japin Li <japinli(at)hotmail(dot)com> wrote:
>
> On Wed, 14 Jan 2026 at 15:26, Roman Khapov <rkhapov(at)yandex-team(dot)ru> wrote:
> >> Hi! Thank you 🙏 for your effort. 0003 looks good to me
> >>
> >> --
> >> Best regards,
> >> Kirill Reshke
> >
> > Hi! Thanks for the patch!
> >
> > I'v been reviewing your patch, and noticed there are some code logic
> > that handles NULL values, but tests doesn't cover this scenarios.
> >
> > So, I added simple line at contrib/pageinspect/sql/gin.sql:
> >
> > INSERT INTO test1 VALUES (1, ARRAY[11, 111], ARRAY['a', 'b', 'c']);
> > +INSERT INTO test1 VALUES (1, ARRAY[NULL, 222], ARRAY['d', NULL]);
> > CREATE INDEX test1_y_idx ON test1 USING gin (y) WITH (fastupdate = off);
> >
> > And got unexpected result...
> > As far as I understand, we shouldn't get error for the whole
> > gin_entrypage_items executions when there are NULL-values columns at index key, but the
> > output contains only the next error:
> >
> > SELECT * FROM gin_entrypage_items(get_raw_page('test1_y_idx', 1), 'test1_y_idx'::regclass);
> > --[ RECORD 1 ]--------------
> > -itemoffset | 1
> > -downlink | (2147483664,1)
> > -tids | {"(0,1)"}
> > -keys | y=11
> > --[ RECORD 2 ]--------------
> > -itemoffset | 2
> > -downlink | (2147483664,1)
> > -tids | {"(0,1)"}
> > -keys | y=111
> > -
> > +ERROR: invalid gin entry page tuple at offset 4
> >
> > Is the NULL values handle correct?
> >
>
> Nice catch!
>
> I agree — the NULL handling seems wrong.
> Here's a quick patch to fix it. What do you think?
>
> --
> Regards,
> Japin Li
> ChengDu WenWu Information Technology Co., Ltd.
>
Yes, simply removing elog here is correct. See also
gintuple_get_attrnum - this is where is copied this part from.
Will you post a new patch version with this applied?
--
Best regards,
Kirill Reshke
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Roman Khapov | 2026-01-14 12:44:32 | Re: GIN pageinspect support for entry tree and posting tree |
| Previous Message | Japin Li | 2026-01-14 12:40:13 | Re: Add IS_INDEX macro to brin and gist index |