Converting contrib SQL functions to new style - Mailing list pgsql-hackers

Attached are some draft patches to convert almost all of the
contrib modules' SQL functions to use SQL-standard function bodies.
The point of this is to remove the residual search_path security
hazards that we couldn't fix in commits 7eeb1d986 et al.  Since
a SQL-style function body is fully parsed at creation time,
its object references are not subject to capture by the run-time
search path.  Possibly there are small performance benefits too,
though I've not tried to measure that.

I've not touched the documentation yet.  I suppose that we can
tone down the warnings added by 7eeb1d986 quite a bit, maybe
replacing them with just "be sure to use version x.y or later".
However I think we may still need an assumption that earthdistance
and cube are in the same schema --- any comments on that?

I'd like to propose squeezing these changes into v14, even though
we're past feature freeze.  Reason one is that this is less a
new feature than a security fix; reason two is that this provides
some non-artificial test coverage for the SQL-function-body feature.

BTW, there still remain a couple of old-style SQL functions in
contrib/adminpack and contrib/lo.  AFAICS those are unconditionally
secure, so I didn't bother with them.

Thoughts?

            regards, tom lane

diff --git a/contrib/citext/Makefile b/contrib/citext/Makefile
index a7de52928d..d879f4eb0b 100644
--- a/contrib/citext/Makefile
+++ b/contrib/citext/Makefile
@@ -4,6 +4,7 @@ MODULES = citext

 EXTENSION = citext
 DATA = citext--1.4.sql \
+    citext--1.6--1.7.sql \
     citext--1.5--1.6.sql \
     citext--1.4--1.5.sql \
     citext--1.3--1.4.sql \
diff --git a/contrib/citext/citext--1.6--1.7.sql b/contrib/citext/citext--1.6--1.7.sql
new file mode 100644
index 0000000000..51a7edf2bc
--- /dev/null
+++ b/contrib/citext/citext--1.6--1.7.sql
@@ -0,0 +1,65 @@
+/* contrib/citext/citext--1.6--1.7.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION citext UPDATE TO '1.7'" to load this file. \quit
+
+--
+-- Matching citext in string comparison functions.
+-- XXX TODO Ideally these would be implemented in C.
+--
+
+CREATE OR REPLACE FUNCTION regexp_match( citext, citext ) RETURNS TEXT[]
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.regexp_match( $1::pg_catalog.text, $2::pg_catalog.text, 'i' );
+
+CREATE OR REPLACE FUNCTION regexp_match( citext, citext, text ) RETURNS TEXT[]
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.regexp_match( $1::pg_catalog.text, $2::pg_catalog.text, CASE WHEN pg_catalog.strpos($3, 'c') = 0
THEN $3 || 'i' ELSE $3 END ); 
+
+CREATE OR REPLACE FUNCTION regexp_matches( citext, citext ) RETURNS SETOF TEXT[]
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE ROWS 1
+RETURN pg_catalog.regexp_matches( $1::pg_catalog.text, $2::pg_catalog.text, 'i' );
+
+CREATE OR REPLACE FUNCTION regexp_matches( citext, citext, text ) RETURNS SETOF TEXT[]
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE ROWS 10
+RETURN pg_catalog.regexp_matches( $1::pg_catalog.text, $2::pg_catalog.text, CASE WHEN pg_catalog.strpos($3, 'c') = 0
THEN $3 || 'i' ELSE $3 END ); 
+
+CREATE OR REPLACE FUNCTION regexp_replace( citext, citext, text ) returns TEXT
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.regexp_replace( $1::pg_catalog.text, $2::pg_catalog.text, $3, 'i');
+
+CREATE OR REPLACE FUNCTION regexp_replace( citext, citext, text, text ) returns TEXT
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.regexp_replace( $1::pg_catalog.text, $2::pg_catalog.text, $3, CASE WHEN pg_catalog.strpos($4, 'c') =
0THEN  $4 || 'i' ELSE $4 END); 
+
+CREATE OR REPLACE FUNCTION regexp_split_to_array( citext, citext ) RETURNS TEXT[]
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.regexp_split_to_array( $1::pg_catalog.text, $2::pg_catalog.text, 'i' );
+
+CREATE OR REPLACE FUNCTION regexp_split_to_array( citext, citext, text ) RETURNS TEXT[]
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.regexp_split_to_array( $1::pg_catalog.text, $2::pg_catalog.text, CASE WHEN pg_catalog.strpos($3,
'c')= 0 THEN  $3 || 'i' ELSE $3 END ); 
+
+CREATE OR REPLACE FUNCTION regexp_split_to_table( citext, citext ) RETURNS SETOF TEXT
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.regexp_split_to_table( $1::pg_catalog.text, $2::pg_catalog.text, 'i' );
+
+CREATE OR REPLACE FUNCTION regexp_split_to_table( citext, citext, text ) RETURNS SETOF TEXT
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.regexp_split_to_table( $1::pg_catalog.text, $2::pg_catalog.text, CASE WHEN pg_catalog.strpos($3,
'c')= 0 THEN  $3 || 'i' ELSE $3 END ); 
+
+CREATE OR REPLACE FUNCTION strpos( citext, citext ) RETURNS INT
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.strpos( pg_catalog.lower( $1::pg_catalog.text ), pg_catalog.lower( $2::pg_catalog.text ) );
+
+CREATE OR REPLACE FUNCTION replace( citext, citext, citext ) RETURNS TEXT
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.regexp_replace( $1::pg_catalog.text, pg_catalog.regexp_replace($2::pg_catalog.text,
'([^a-zA-Z_0-9])',E'\\\\\\1', 'g'), $3::pg_catalog.text, 'gi' ); 
+
+CREATE OR REPLACE FUNCTION split_part( citext, citext, int ) RETURNS TEXT
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN (pg_catalog.regexp_split_to_array( $1::pg_catalog.text, pg_catalog.regexp_replace($2::pg_catalog.text,
'([^a-zA-Z_0-9])',E'\\\\\\1', 'g'), 'i'))[$3]; 
+
+CREATE OR REPLACE FUNCTION translate( citext, citext, text ) RETURNS TEXT
+LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE
+RETURN pg_catalog.translate( pg_catalog.translate( $1::pg_catalog.text, pg_catalog.lower($2::pg_catalog.text), $3),
pg_catalog.upper($2::pg_catalog.text),$3); 
diff --git a/contrib/citext/citext.control b/contrib/citext/citext.control
index ccf445475d..f82265b334 100644
--- a/contrib/citext/citext.control
+++ b/contrib/citext/citext.control
@@ -1,6 +1,6 @@
 # citext extension
 comment = 'data type for case-insensitive character strings'
-default_version = '1.6'
+default_version = '1.7'
 module_pathname = '$libdir/citext'
 relocatable = true
 trusted = true
diff --git a/contrib/earthdistance/Makefile b/contrib/earthdistance/Makefile
index f93b7a925a..0cf3fa379a 100644
--- a/contrib/earthdistance/Makefile
+++ b/contrib/earthdistance/Makefile
@@ -3,7 +3,8 @@
 MODULES = earthdistance

 EXTENSION = earthdistance
-DATA = earthdistance--1.1.sql earthdistance--1.0--1.1.sql
+DATA = earthdistance--1.1.sql earthdistance--1.0--1.1.sql \
+    earthdistance--1.1--1.2.sql
 PGFILEDESC = "earthdistance - calculate distances on the surface of the Earth"

 REGRESS = earthdistance
diff --git a/contrib/earthdistance/earthdistance--1.1--1.2.sql b/contrib/earthdistance/earthdistance--1.1--1.2.sql
new file mode 100644
index 0000000000..7c895e2204
--- /dev/null
+++ b/contrib/earthdistance/earthdistance--1.1--1.2.sql
@@ -0,0 +1,57 @@
+/* contrib/earthdistance/earthdistance--1.1--1.2.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION earthdistance UPDATE TO '1.2'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION earth() RETURNS float8
+LANGUAGE SQL IMMUTABLE PARALLEL SAFE
+RETURN '6378168'::float8;
+
+CREATE OR REPLACE FUNCTION sec_to_gc(float8)
+RETURNS float8
+LANGUAGE SQL
+IMMUTABLE STRICT
+PARALLEL SAFE
+RETURN CASE WHEN $1 < 0 THEN 0::float8 WHEN $1/(2*earth()) > 1 THEN pi()*earth() ELSE 2*earth()*asin($1/(2*earth()))
END;
+
+CREATE OR REPLACE FUNCTION gc_to_sec(float8)
+RETURNS float8
+LANGUAGE SQL
+IMMUTABLE STRICT
+PARALLEL SAFE
+RETURN CASE WHEN $1 < 0 THEN 0::float8 WHEN $1/earth() > pi() THEN 2*earth() ELSE 2*earth()*sin($1/(2*earth())) END;
+
+CREATE OR REPLACE FUNCTION ll_to_earth(float8, float8)
+RETURNS earth
+LANGUAGE SQL
+IMMUTABLE STRICT
+PARALLEL SAFE
+RETURN
cube(cube(cube(earth()*cos(radians($1))*cos(radians($2))),earth()*cos(radians($1))*sin(radians($2))),earth()*sin(radians($1)))::earth;
+
+CREATE OR REPLACE FUNCTION latitude(earth)
+RETURNS float8
+LANGUAGE SQL
+IMMUTABLE STRICT
+PARALLEL SAFE
+RETURN CASE WHEN cube_ll_coord($1, 3)/earth() < -1 THEN -90::float8 WHEN cube_ll_coord($1, 3)/earth() > 1 THEN
90::float8ELSE degrees(asin(cube_ll_coord($1, 3)/earth())) END; 
+
+CREATE OR REPLACE FUNCTION longitude(earth)
+RETURNS float8
+LANGUAGE SQL
+IMMUTABLE STRICT
+PARALLEL SAFE
+RETURN degrees(atan2(cube_ll_coord($1, 2), cube_ll_coord($1, 1)));
+
+CREATE OR REPLACE FUNCTION earth_distance(earth, earth)
+RETURNS float8
+LANGUAGE SQL
+IMMUTABLE STRICT
+PARALLEL SAFE
+RETURN sec_to_gc(cube_distance($1, $2));
+
+CREATE OR REPLACE FUNCTION earth_box(earth, float8)
+RETURNS cube
+LANGUAGE SQL
+IMMUTABLE STRICT
+PARALLEL SAFE
+RETURN cube_enlarge($1, gc_to_sec($2), 3);
diff --git a/contrib/earthdistance/earthdistance.control b/contrib/earthdistance/earthdistance.control
index 5816d22cdd..de2465d487 100644
--- a/contrib/earthdistance/earthdistance.control
+++ b/contrib/earthdistance/earthdistance.control
@@ -1,6 +1,6 @@
 # earthdistance extension
 comment = 'calculate great-circle distances on the surface of the Earth'
-default_version = '1.1'
+default_version = '1.2'
 module_pathname = '$libdir/earthdistance'
 relocatable = true
 requires = 'cube'
diff --git a/contrib/pageinspect/pageinspect--1.8--1.9.sql b/contrib/pageinspect/pageinspect--1.8--1.9.sql
index be89a64ca1..4077f9ecee 100644
--- a/contrib/pageinspect/pageinspect--1.8--1.9.sql
+++ b/contrib/pageinspect/pageinspect--1.8--1.9.sql
@@ -135,3 +135,73 @@ CREATE FUNCTION brin_page_items(IN page bytea, IN index_oid regclass,
 RETURNS SETOF record
 AS 'MODULE_PATHNAME', 'brin_page_items'
 LANGUAGE C STRICT PARALLEL SAFE;
+
+-- Convert SQL functions to new style
+
+CREATE OR REPLACE FUNCTION heap_page_item_attrs(
+    IN page bytea,
+    IN rel_oid regclass,
+    IN do_detoast bool,
+    OUT lp smallint,
+    OUT lp_off smallint,
+    OUT lp_flags smallint,
+    OUT lp_len smallint,
+    OUT t_xmin xid,
+    OUT t_xmax xid,
+    OUT t_field3 int4,
+    OUT t_ctid tid,
+    OUT t_infomask2 integer,
+    OUT t_infomask integer,
+    OUT t_hoff smallint,
+    OUT t_bits text,
+    OUT t_oid oid,
+    OUT t_attrs bytea[]
+    )
+RETURNS SETOF record
+LANGUAGE SQL PARALLEL SAFE
+BEGIN ATOMIC
+SELECT lp,
+       lp_off,
+       lp_flags,
+       lp_len,
+       t_xmin,
+       t_xmax,
+       t_field3,
+       t_ctid,
+       t_infomask2,
+       t_infomask,
+       t_hoff,
+       t_bits,
+       t_oid,
+       tuple_data_split(
+         rel_oid,
+         t_data,
+         t_infomask,
+         t_infomask2,
+         t_bits,
+         do_detoast)
+         AS t_attrs
+  FROM heap_page_items(page);
+END;
+
+CREATE OR REPLACE FUNCTION heap_page_item_attrs(IN page bytea, IN rel_oid regclass,
+    OUT lp smallint,
+    OUT lp_off smallint,
+    OUT lp_flags smallint,
+    OUT lp_len smallint,
+    OUT t_xmin xid,
+    OUT t_xmax xid,
+    OUT t_field3 int4,
+    OUT t_ctid tid,
+    OUT t_infomask2 integer,
+    OUT t_infomask integer,
+    OUT t_hoff smallint,
+    OUT t_bits text,
+    OUT t_oid oid,
+    OUT t_attrs bytea[]
+    )
+RETURNS SETOF record
+LANGUAGE SQL PARALLEL SAFE
+BEGIN ATOMIC
+SELECT * FROM heap_page_item_attrs(page, rel_oid, false);
+END;
diff --git a/contrib/pg_freespacemap/Makefile b/contrib/pg_freespacemap/Makefile
index da40b80c7c..e09fa7ccbe 100644
--- a/contrib/pg_freespacemap/Makefile
+++ b/contrib/pg_freespacemap/Makefile
@@ -6,7 +6,9 @@ OBJS = \
     pg_freespacemap.o

 EXTENSION = pg_freespacemap
-DATA = pg_freespacemap--1.1.sql pg_freespacemap--1.1--1.2.sql \
+DATA = pg_freespacemap--1.1.sql \
+    pg_freespacemap--1.2--1.3.sql \
+    pg_freespacemap--1.1--1.2.sql \
     pg_freespacemap--1.0--1.1.sql
 PGFILEDESC = "pg_freespacemap - monitoring of free space map"

diff --git a/contrib/pg_freespacemap/pg_freespacemap--1.2--1.3.sql
b/contrib/pg_freespacemap/pg_freespacemap--1.2--1.3.sql
new file mode 100644
index 0000000000..7f92c9e92e
--- /dev/null
+++ b/contrib/pg_freespacemap/pg_freespacemap--1.2--1.3.sql
@@ -0,0 +1,13 @@
+/* contrib/pg_freespacemap/pg_freespacemap--1.2--1.3.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pg_freespacemap UPDATE TO '1.3'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION
+  pg_freespace(rel regclass, blkno OUT bigint, avail OUT int2)
+RETURNS SETOF RECORD
+LANGUAGE SQL PARALLEL SAFE
+BEGIN ATOMIC
+  SELECT blkno, pg_freespace($1, blkno) AS avail
+  FROM generate_series(0, pg_relation_size($1) / current_setting('block_size')::bigint - 1) AS blkno;
+END;
diff --git a/contrib/pg_freespacemap/pg_freespacemap.control b/contrib/pg_freespacemap/pg_freespacemap.control
index ac8fc5050a..1992320691 100644
--- a/contrib/pg_freespacemap/pg_freespacemap.control
+++ b/contrib/pg_freespacemap/pg_freespacemap.control
@@ -1,5 +1,5 @@
 # pg_freespacemap extension
 comment = 'examine the free space map (FSM)'
-default_version = '1.2'
+default_version = '1.3'
 module_pathname = '$libdir/pg_freespacemap'
 relocatable = true
diff --git a/contrib/xml2/Makefile b/contrib/xml2/Makefile
index 0d703fe0e8..8597e9aa9c 100644
--- a/contrib/xml2/Makefile
+++ b/contrib/xml2/Makefile
@@ -7,7 +7,9 @@ OBJS = \
     xslt_proc.o
 
 EXTENSION = xml2
-DATA = xml2--1.1.sql xml2--1.0--1.1.sql
+DATA = xml2--1.1.sql \
+    xml2--1.1--1.2.sql \
+    xml2--1.0--1.1.sql
 PGFILEDESC = "xml2 - XPath querying and XSLT"
 
 REGRESS = xml2
diff --git a/contrib/xml2/xml2--1.1--1.2.sql b/contrib/xml2/xml2--1.1--1.2.sql
new file mode 100644
index 0000000000..12d00064e8
--- /dev/null
+++ b/contrib/xml2/xml2--1.1--1.2.sql
@@ -0,0 +1,18 @@
+/* contrib/xml2/xml2--1.1--1.2.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION xml2 UPDATE TO '1.2'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION xpath_list(text,text) RETURNS text
+LANGUAGE SQL STRICT IMMUTABLE PARALLEL SAFE
+RETURN xpath_list($1,$2,',');
+
+CREATE OR REPLACE FUNCTION xpath_nodeset(text,text)
+RETURNS text
+LANGUAGE SQL STRICT IMMUTABLE PARALLEL SAFE
+RETURN xpath_nodeset($1,$2,'','');
+
+CREATE OR REPLACE FUNCTION xpath_nodeset(text,text,text)
+RETURNS text
+LANGUAGE SQL STRICT IMMUTABLE PARALLEL SAFE
+RETURN xpath_nodeset($1,$2,'',$3);
diff --git a/contrib/xml2/xml2.control b/contrib/xml2/xml2.control
index ba2c0599a3..b32156c949 100644
--- a/contrib/xml2/xml2.control
+++ b/contrib/xml2/xml2.control
@@ -1,6 +1,6 @@
 # xml2 extension
 comment = 'XPath querying and XSLT'
-default_version = '1.1'
+default_version = '1.2'
 module_pathname = '$libdir/pgxml'
 # XXX do we still need this to be non-relocatable?
 relocatable = false