Re: [WIP]Vertical Clustered Index (columnar store extension) - take2

On Thu, Jul 10, 2025 at 4:07 PM Japin Li <japinli@hotmail.com> wrote:
...
> Thanks for updating the patch!
>
> I find another bug when using VCI index as following:
>
> 2025-07-10 12:17:15.087 CST [2027312] PANIC:  unexpected index access method call : "vci_beginscan"
> 2025-07-10 12:17:15.087 CST [2027312] STATEMENT:  SELECT * FROM t WHERE id = 100;
> 2025-07-10 12:17:15.228 CST [2027143] LOG:  client backend (PID 2027312) was terminated by signal 6: Aborted
> 2025-07-10 12:17:15.228 CST [2027143] DETAIL:  Failed process was running: SELECT * FROM t WHERE id = 100;
> 2025-07-10 12:17:15.228 CST [2027143] LOG:  terminating any other active server processes
> 2025-07-10 12:17:15.231 CST [2027143] LOG:  all server processes terminated; reinitializing
> 2025-07-10 12:17:15.257 CST [2027317] LOG:  database system was interrupted; last known up at 2025-07-10 12:17:07 CST
> 2025-07-10 12:17:15.333 CST [2027317] LOG:  database system was not properly shut down; automatic recovery in progress
> 2025-07-10 12:17:15.337 CST [2027317] LOG:  redo starts at 0/01761A18
> 2025-07-10 12:17:15.491 CST [2027143] LOG:  startup process (PID 2027317) was terminated by signal 11: Segmentation fault
> 2025-07-10 12:17:15.491 CST [2027143] LOG:  terminating any other active server processes
> 2025-07-10 12:17:15.493 CST [2027143] LOG:  shutting down due to startup process failure
> 2025-07-10 12:17:15.496 CST [2027143] LOG:  database system is shut down
>
> Here are the steps:
>
> rm -rf demo/
> initdb -D demo
> cat <<EOF >demo/postgresql.auto.conf
> shared_preload_libraries = 'vci'
> max_worker_processes = '20'
> EOF
>
> pg_ctl -D demo -l demo.log start
>
> cat <<EOF | psql postgres
> CREATE EXTENSION vci;
> CREATE TABLE t (id int, info text);
> CREATE INDEX t_id_idx ON t USING vci (id);
> INSERT INTO t SELECT id, md5(random()::text) FROM generate_series(1, 1000) id;
> SET enable_seqscan TO off;
> SELECT * FROM t WHERE id = 100;
> EOF
>

Thanks for reporting this. We will look into it as soon as we can.

> I'm still trying to understand the patches.
>
> diff --git a/src/include/storage/itemptr.h b/src/include/storage/itemptr.h
> index 74b87a9..d97d1c5 100644
> --- a/src/include/storage/itemptr.h
> +++ b/src/include/storage/itemptr.h
> @@ -46,6 +46,9 @@ typedef struct ItemPointerData
>  #endif
>  ItemPointerData;
>
> +#define SizeOfIptrData \
> +               (offsetof(ItemPointerData, ip_posid) + sizeof(OffsetNumber))
> +
>
> I've noticed this macro is currently defined within core; however, I found it only
> used in the VCI extension.
>
> Could you clarify the rationale for its inclusion in the core, and whether it's
> genuinely required there, or if it would be better suited within the extension
> itself?

Right, this had previously also been reported by Tomas [1].

Upon investigation, I found that this was master code from 10 years
ago (back when this VCI patch was implemented). The master code has
moved on since then and removed this macro [2], but this VCI patch did
not...

I'll try to address this for the next patchset.

======
[1] https://www.postgresql.org/message-id/a748aa6b-c7e6-4d02-a590-ab404d590448%40vondra.me
[2] https://github.com/postgres/postgres/commit/8023b5827fbada6815ce269db4f3373ac77ec7c3

Kind Regards,
Peter Smith.
Fujitsu Australia