On Thu, 2025-06-19 at 14:14 +1000, Peter Smith wrote:
> Here are the v8 patches. The main changes are as follows:
> 
> v8-0001 VCI - changes to postgres core
> - same
> 
> v8-0002 VCI module - main
> - extracted "compression" related code from this main patch
> - applied Timur's top-up patch [1] re "session_preload_libraries"
> - removed some dead code
> 
> v8-0003 VCI module - documentation
> - removed mentions about compression

Hello Peter!

Thank you for working on VCI updates.
Here are some proposals for small improvements:

Since hothash feature lives in separate patch now, vci_hothash.o should
be removed from vci/executor/Makefile of VCI-module-main patch.

0001-Avoid-magic-numbers-in-vci_is_supported_type.patch
I've looked at vci_supported_types.c and tried to rewrite it without
using magic constants. Removed call to vci_check_supported_types() from
SQL code since there is no need now to check that OID constants still
match same types.
Using defined constants for OIDs seems more robust than plain numbers.
There were 23 type OIDs supported by VCI, all of them are there in a
new version of code. I've replaced binary search by simple switch-case
since compilers optimize it into nice binary decision tree. Previous
version was binary searching among 87 structs. New version only
searches among 23 integers.

0002-Updated-supported-funcs-SQL-for-recent-PostgreSQL.patch
I wonder if it is possible to rewrite vci_supported_funcs.c in a
similar way. There is a vci_supported_funcs.sql which I updated so it
can be executed at master version of PostgreSQL.
I don't know if it is intentional, but safe_types list from
vci_supported_funcs.sql have some differences to the types list from
vci_supported_types.c. Specifically, it doesn't include varchar, varbit
and uuid.

-- 
Regards,
Timur Magomedov

From 440161877a113a233d54b5f83ec9764682e151e3 Mon Sep 17 00:00:00 2001
From: Timur Magomedov <t.magome...@postgrespro.ru>
Date: Tue, 17 Jun 2025 19:04:17 +0300
Subject: [PATCH 1/2] Avoid magic numbers in vci_is_supported_type()

---
 contrib/vci/vci--2.0.sql          |   6 -
 contrib/vci/vci_supported_types.c | 230 +++++-------------------------
 2 files changed, 34 insertions(+), 202 deletions(-)

diff --git a/contrib/vci/vci--2.0.sql b/contrib/vci/vci--2.0.sql
index 4ba6c2d416e..7244de168b4 100644
--- a/contrib/vci/vci--2.0.sql
+++ b/contrib/vci/vci--2.0.sql
@@ -36,11 +36,6 @@ RETURNS void
 AS 'MODULE_PATHNAME'
 LANGUAGE C STABLE STRICT;
 
-CREATE FUNCTION vci_check_supported_types()
-RETURNS void
-AS 'MODULE_PATHNAME'
-LANGUAGE C STABLE STRICT;
-
 CREATE FUNCTION vci_index_size(IN vci_index_name text, OUT size int8)
 AS 'MODULE_PATHNAME'
 LANGUAGE C VOLATILE STRICT;
@@ -73,4 +68,3 @@ AS 'MODULE_PATHNAME'
 LANGUAGE C VOLATILE STRICT;
 
 SELECT vci_check_supported_functions();
-SELECT vci_check_supported_types();
diff --git a/contrib/vci/vci_supported_types.c b/contrib/vci/vci_supported_types.c
index 1a755b879d5..29e4d0d5c48 100644
--- a/contrib/vci/vci_supported_types.c
+++ b/contrib/vci/vci_supported_types.c
@@ -3,9 +3,13 @@
  * vci_supported_types.c
  *	  Types supported by VCI
  *
- * vci_supported_type_table[] is created with following SQL and then examined individually.
+ * List of possibly supported types can be queried with following SQL and then
+ * examined individually:
  *
- *   SELECT oid, typname FROM pg_type WHERE typnamespace = 11 AND typrelid = 0 AND typelem = 0 ORDER BY oid;
+ *   SELECT oid, typname
+ *   FROM pg_type
+ *   WHERE typnamespace = 11 AND typrelid = 0 AND typelem = 0
+ *   ORDER BY oid;
  *
  * - 'typnamespace = 11' is to exclude types not related to table structure
  * - 'typelem = 0' is to exclude array type
@@ -18,127 +22,11 @@
  */
 #include "postgres.h"
 
-#include "access/heapam.h"
-#include "access/htup.h"
-#include "access/htup_details.h"
-#include "c.h"
-#include "catalog/pg_type.h"	/* for TypeRelationId, Form_pg_type */
-#include "fmgr.h"
-#include "utils/elog.h"
-#include "utils/relcache.h"
-#include "utils/syscache.h"
+#include "catalog/pg_type_d.h"	/* for type oid constants */
 
 #include "vci.h"
 #include "vci_supported_oid.h"
 
-/**
- * Smallest OID among types supported by VCI
- */
-#define VCI_SUPPORTED_TYPE_MIN (16)
-
-/**
- * Biggest OID among types supported by VCI
- */
-#define VCI_SUPPORTED_TYPE_MAX (7001)
-
-/**
- * Array of information about types supported by VCI
- */
-static const struct
-{
-	Oid			oid;
-	const char *name;
-	bool		is_support;
-}			vci_supported_type_table[] = {
-	{16, "bool", true},			/* BOOLOID */
-	{17, "bytea", true},		/* BYTEAOID */
-	{18, "char", true},			/* CHAROID */
-	{19, "name", true},			/* NAMEOID */
-	{20, "int8", true},			/* INT8OID */
-	{21, "int2", true},			/* INT2OID */
-	{23, "int4", true},			/* INT4OID */
-	{24, "regproc", false},		/* REGPROCOID */
-	{25, "text", true},			/* TEXTOID */
-	{26, "oid", false},			/* OIDOID */
-	{27, "tid", false},			/* TIDOID */
-	{28, "xid", false},			/* XIDOID */
-	{29, "cid", false},			/* CIDOID */
-	{114, "json", false},		/* JSONOID */
-	{142, "xml", false},		/* XMLOID */
-	{194, "pg_node_tree", false},	/* PG_NODE_TREEOID */
-	{602, "path", false},		/* PATHOID */
-	{604, "polygon", false},	/* POLYGONOID */
-	{650, "cidr", false},		/* CIDROID */
-	{700, "float4", true},		/* FLOAT4OID */
-	{701, "float8", true},		/* FLOAT8OID */
-	{705, "unknown", false},	/* UNKNOWNOID */
-	{718, "circle", false},		/* CIRCLEOID */
-	{790, "money", true},		/* MONEYOID */
-	{829, "macaddr", false},	/* MACADDROID */
-	{869, "inet", false},		/* INETOID */
-	{1033, "aclitem", false},	/* ACLITEMOID */
-	{1042, "bpchar", true},		/* BPCHAROID */
-	{1043, "varchar", true},	/* VARCHAROID */
-	{1082, "date", true},		/* DATEOID */
-	{1083, "time", true},		/* TIMEOID */
-	{1114, "timestamp", true},	/* TIMESTAMPOID */
-	{1184, "timestamptz", true},	/* TIMESTAMPTZOID */
-	{1186, "interval", true},	/* INTERVALOID */
-	{1266, "timetz", true},		/* TIMETZOID */
-	{1560, "bit", true},		/* BITOID */
-	{1562, "varbit", true},		/* VARBITOID */
-	{1700, "numeric", true},	/* NUMERICOID */
-	{1790, "refcursor", false}, /* REFCURSOROID */
-	{2202, "regprocedure", false},	/* REGPROCEDUREOID */
-	{2203, "regoper", false},	/* REGOPEROID */
-	{2204, "regoperator", false},	/* REGOPERATOROID */
-	{2205, "regclass", false},	/* REGCLASSOID */
-	{2206, "regtype", false},	/* REGTYPEOID */
-	{2249, "record", false},	/* RECORDOID */
-	{2275, "cstring", false},	/* CSTRINGOID */
-	{2276, "any", false},		/* ANYOID */
-	{2277, "anyarray", false},	/* ANYARRAYOID */
-	{2278, "void", false},		/* VOIDOID */
-	{2279, "trigger", false},	/* TRIGGEROID */
-	{2280, "language_handler", false},	/* LANGUAGE_HANDLEROID */
-	{2281, "internal", false},	/* INTERNALOID */
-	{2283, "anyelement", false},	/* ANYELEMENTOID */
-	{2776, "anynonarray", false},	/* ANYNONARRAYOID */
-	{2950, "uuid", true},		/* UUIDOID */
-	{2970, "txid_snapshot", false},
-	{3115, "fdw_handler", false},	/* FDW_HANDLEROID */
-	{3220, "pg_lsn", false},	/* PG_LSNOID */
-	{3500, "anyenum", false},	/* ANYENUMOID */
-	{3614, "tsvector", false},	/* TSVECTOROID */
-	{3615, "tsquery", false},	/* TSQUERYOID */
-	{3642, "gtsvector", false}, /* GTSVECTOROID */
-	{3734, "regconfig", false}, /* REGCONFIGOID */
-	{3769, "regdictionary", false}, /* REGDICTIONARYOID */
-	{3802, "jsonb", false},		/* JSONBOID */
-	{3831, "anyrange", false},	/* ANYRANGEOID */
-	{3838, "event_trigger", false}, /* EVENT_TRIGGEROID */
-	{3904, "int4range", false}, /* INT4RANGEOID */
-	{3906, "numrange", false},
-	{3908, "tsrange", false},
-	{3910, "tstzrange", false},
-	{3912, "daterange", false},
-	{3926, "int8range", false},
-	{4451, "int4multirange", false},
-	{4532, "nummultirange", false},
-	{4533, "tsmultirange", false},
-	{4534, "tstzmultirange", false},
-	{4535, "datemultirange", false},
-	{4536, "int8multirange", false},
-	{4537, "anymultirange", false},
-	{4538, "anycompatiblemultirange", false},
-	{5038, "pg_snapshot", false},	/* PG_SNAPSHOTOID */
-	{5069, "xid8", false},		/* XID8OID */
-	{5077, "anycompatible", false}, /* ANYCOMPATIBLEOID */
-	{5078, "anycompatiblearray", false},	/* ANYCOMPATIBLEARRAYOID */
-	{5079, "anycompatiblenonarray", false}, /* ANYCOMPATIBLENONARRAYOID */
-	{5080, "anycompatiblerange", false},	/* ANYCOMPATIBLERANGEOID */
-};
-
 /**
  * Determine if the given oid is a type that can be supported by VCI
  *
@@ -148,83 +36,33 @@ static const struct
 bool
 vci_is_supported_type(Oid oid)
 {
-	int			min,
-				max,
-				pivot;
-
-	if ((oid < VCI_SUPPORTED_TYPE_MIN) || (VCI_SUPPORTED_TYPE_MAX < oid))
-		return false;
-
-	/* 2 minute search */
-
-	min = 0;
-	max = lengthof(vci_supported_type_table);	/* exclusive */
-
-	while (max - min > 1)
+	switch (oid)
 	{
-		Oid			comp;
-
-		pivot = (min + max) / 2;
-
-		comp = vci_supported_type_table[pivot].oid;
-
-		if (comp == oid)
-			return vci_supported_type_table[pivot].is_support;
-		else if (oid < comp)
-			max = pivot;
-		else					/* comp < oid */
-			min = pivot;
+		case BOOLOID:
+		case BYTEAOID:
+		case CHAROID:
+		case NAMEOID:
+		case INT8OID:
+		case INT2OID:
+		case INT4OID:
+		case TEXTOID:
+		case FLOAT4OID:
+		case FLOAT8OID:
+		case MONEYOID:
+		case BPCHAROID:
+		case VARCHAROID:
+		case DATEOID:
+		case TIMEOID:
+		case TIMESTAMPOID:
+		case TIMESTAMPTZOID:
+		case INTERVALOID:
+		case TIMETZOID:
+		case BITOID:
+		case VARBITOID:
+		case NUMERICOID:
+		case UUIDOID:
+			return true;
+		default:
+			return false;
 	}
-
-	if (max - min == 1)
-		if (oid == vci_supported_type_table[min].oid)
-			return vci_supported_type_table[min].is_support;
-
-	return false;
-}
-
-/*==========================================================================*/
-/* Implementation of PG function to check supported types at CREATE EXTENSION */
-/*==========================================================================*/
-
-PG_FUNCTION_INFO_V1(vci_check_supported_types);
-
-Datum
-vci_check_supported_types(PG_FUNCTION_ARGS)
-{
-	Relation	rel;
-	int			i;
-
-	rel = table_open(TypeRelationId, AccessShareLock);
-
-	for (i = 0; i < lengthof(vci_supported_type_table); i++)
-	{
-		HeapTuple	tuple;
-		Form_pg_type typeform;
-
-		if (!vci_supported_type_table[i].is_support)
-			continue;
-
-		tuple = SearchSysCache1(TYPEOID, ObjectIdGetDatum(vci_supported_type_table[i].oid));
-		if (!HeapTupleIsValid(tuple))
-			goto error;
-
-		typeform = (Form_pg_type) GETSTRUCT(tuple);
-
-		if (strcmp(vci_supported_type_table[i].name, NameStr(typeform->typname)) != 0)
-			goto error;
-
-		ReleaseSysCache(tuple);
-	}
-
-	table_close(rel, AccessShareLock);
-
-	PG_RETURN_VOID();
-
-error:
-	ereport(ERROR,
-			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-			 errmsg("extension \"%s\" cannot be installed under this version of PostgreSQL", VCI_STRING)));
-
-	PG_RETURN_VOID();
 }
-- 
2.43.0

From 87b70df29458dab7dfdeac0f7ac1de975e625587 Mon Sep 17 00:00:00 2001
From: Timur Magomedov <t.magome...@postgrespro.ru>
Date: Thu, 19 Jun 2025 19:55:53 +0300
Subject: [PATCH 2/2] Updated supported funcs SQL for recent PostgreSQL

---
 contrib/vci/vci_supported_funcs.sql | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/vci/vci_supported_funcs.sql b/contrib/vci/vci_supported_funcs.sql
index b80c53085e6..4761173cbf2 100644
--- a/contrib/vci/vci_supported_funcs.sql
+++ b/contrib/vci/vci_supported_funcs.sql
@@ -2,7 +2,7 @@
 
 CREATE TEMPORARY TABLE test (funcoid oid);
 INSERT INTO test (funcoid) SELECT unnest(ARRAY[aggfnoid, aggtransfn, aggfinalfn, aggmtransfn, aggminvtransfn, aggmfinalfn]) FROM pg_aggregate;
-INSERT INTO test (funcoid) SELECT unnest(ARRAY[aminsert, ambeginscan, amgettuple, amgetbitmap, amrescan, amendscan, ammarkpos, amrestrpos, ambuild, ambuildempty, ambulkdelete, amvacuumcleanup, amcanreturn, amcostestimate, amoptions]) FROM pg_am;
+INSERT INTO test (funcoid) SELECT amhandler FROM pg_am;
 INSERT INTO test (funcoid) SELECT unnest(ARRAY[amproc]) FROM pg_amproc;
 INSERT INTO test (funcoid) SELECT unnest(ARRAY[castfunc]) FROM pg_cast;
 INSERT INTO test (funcoid) SELECT unnest(ARRAY[conproc]) FROM pg_conversion;
@@ -105,7 +105,7 @@ CREATE FUNCTION print_typename(IN oids _oid) RETURNS _name AS $$
 	   SELECT array_agg(pg_type.typname) FROM unnest(oids) AS t(i), pg_type WHERE i = pg_type.oid;
 $$ LANGUAGE SQL;
 
-SELECT oid, proname, provolatile, prolang, print_typename(array_prepend(prorettype, proargtypes)) FROM pg_proc WHERE NOT proisagg AND NOT proiswindow AND NOT proretset
+SELECT oid, proname, provolatile, prolang, print_typename(array_prepend(prorettype, proargtypes)) FROM pg_proc WHERE prokind = 'f' AND NOT proretset
     AND NOT EXISTS (SELECT funcoid FROM sys_func_table WHERE pg_proc.oid = sys_func_table.funcoid)
     AND (SELECT bool_and(i IN (SELECT typeoid FROM safe_types)) FROM unnest(array_prepend(prorettype, proargtypes)) AS t(i))
     AND oid < 16384 ORDER BY oid;
-- 
2.43.0

Reply via email to