Thread: Re: [WIP]Vertical Clustered Index (columnar store extension) - take2
On Tue, 08 Jul 2025 at 10:11, Peter Smith <smithpb2250@gmail.com> wrote: > Hi. Here is the latest patch set. > > Main differences are: > > Patch 0001 (core) > - removed embedded vci code from autovacuum.c / postinit.c (now these > are same as master) > > Patch 0002 (vci module) > - a few changes to facilitate the fix to patch 0001 > - removed file vci.lib > Hi, Peter 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 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? -- Regards, Japin Li
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