From 0edfc4f411b9b1fa5731c5f28d6225256f7077e1 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 30 Jun 2022 10:30:35 +0900
Subject: [PATCH v4] Add an argument variation to pg_read_file

Let the functions pg_read_file and pg_read_binary_file have the
argument variation of f(filename, missing_ok) so that the functions
can read the whole file tolerating the file to be missing.
---
 doc/src/sgml/func.sgml                       |   4 +-
 src/backend/catalog/system_functions.sql     |   4 +
 src/backend/utils/adt/genfile.c              | 205 +++++++++++++------
 src/include/catalog/pg_proc.dat              |  11 +-
 src/test/regress/expected/misc_functions.out |  77 +++++++
 src/test/regress/sql/misc_functions.sql      |  31 +++
 6 files changed, 260 insertions(+), 72 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 21f8ab73e2..49ebf6dea9 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -28636,7 +28636,7 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
         <indexterm>
          <primary>pg_read_file</primary>
         </indexterm>
-        <function>pg_read_file</function> ( <parameter>filename</parameter> <type>text</type> <optional>, <parameter>offset</parameter> <type>bigint</type>, <parameter>length</parameter> <type>bigint</type> <optional>, <parameter>missing_ok</parameter> <type>boolean</type> </optional></optional> )
+        <function>pg_read_file</function> ( <parameter>filename</parameter> <type>text</type> <optional>, <parameter>offset</parameter> <type>bigint</type>, <parameter>length</parameter> <type>bigint</type> </optional> <optional>, <parameter>missing_ok</parameter> <type>boolean</type> </optional> )
         <returnvalue>text</returnvalue>
        </para>
        <para>
@@ -28661,7 +28661,7 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
         <indexterm>
          <primary>pg_read_binary_file</primary>
         </indexterm>
-        <function>pg_read_binary_file</function> ( <parameter>filename</parameter> <type>text</type> <optional>, <parameter>offset</parameter> <type>bigint</type>, <parameter>length</parameter> <type>bigint</type> <optional>, <parameter>missing_ok</parameter> <type>boolean</type> </optional></optional> )
+        <function>pg_read_binary_file</function> ( <parameter>filename</parameter> <type>text</type> <optional>, <parameter>offset</parameter> <type>bigint</type>, <parameter>length</parameter> <type>bigint</type> </optional> <optional>, <parameter>missing_ok</parameter> <type>boolean</type> </optional> )
         <returnvalue>bytea</returnvalue>
        </para>
        <para>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 73da687d5d..30a048f6b0 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -659,12 +659,16 @@ REVOKE EXECUTE ON FUNCTION pg_ls_tmpdir(oid) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_read_file(text) FROM public;
 
+REVOKE EXECUTE ON FUNCTION pg_read_file(text,boolean) FROM public;
+
 REVOKE EXECUTE ON FUNCTION pg_read_file(text,bigint,bigint) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_read_file(text,bigint,bigint,boolean) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text) FROM public;
 
+REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text,boolean) FROM public;
+
 REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text,bigint,bigint) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text,bigint,bigint,boolean) FROM public;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 2bf5219256..7ac6ede42d 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -278,81 +278,49 @@ pg_read_file(PG_FUNCTION_ARGS)
  *
  * No superuser check done here- instead privileges are handled by the
  * GRANT system.
+ *
+ * If read_to_eof is true, bytes_to_read must be -1, otherwise negative values
+ * are not allowed for bytes_to_read.
  */
-Datum
-pg_read_file_v2(PG_FUNCTION_ARGS)
+static text*
+pg_read_file_common(text *filename_t, int64 seek_offset, int64 bytes_to_read,
+					bool read_to_eof, bool missing_ok)
 {
-	text	   *filename_t = PG_GETARG_TEXT_PP(0);
-	int64		seek_offset = 0;
-	int64		bytes_to_read = -1;
-	bool		missing_ok = false;
-	char	   *filename;
-	text	   *result;
-
-	/* handle optional arguments */
-	if (PG_NARGS() >= 3)
-	{
-		seek_offset = PG_GETARG_INT64(1);
-		bytes_to_read = PG_GETARG_INT64(2);
-
-		if (bytes_to_read < 0)
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("requested length cannot be negative")));
-	}
-	if (PG_NARGS() >= 4)
-		missing_ok = PG_GETARG_BOOL(3);
-
-	filename = convert_and_check_filename(filename_t);
-
-	result = read_text_file(filename, seek_offset, bytes_to_read, missing_ok);
-	if (result)
-		PG_RETURN_TEXT_P(result);
-	else
-		PG_RETURN_NULL();
+	if (read_to_eof)
+		Assert(bytes_to_read == -1);
+	else if (bytes_to_read < 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("requested length cannot be negative")));
+
+	return read_text_file(convert_and_check_filename(filename_t),
+						  seek_offset, bytes_to_read, missing_ok);
 }
 
 /*
- * Read a section of a file, returning it as bytea
+ * Read a section of a file, returning it as bytea.  Parameters are interpreted
+ * and restricted the same with pg_read_file_common().
  */
-Datum
-pg_read_binary_file(PG_FUNCTION_ARGS)
+static bytea*
+pg_read_binary_file_common(text *filename_t,
+						   int64 seek_offset, int64 bytes_to_read,
+						   bool read_to_eof, bool missing_ok)
 {
-	text	   *filename_t = PG_GETARG_TEXT_PP(0);
-	int64		seek_offset = 0;
-	int64		bytes_to_read = -1;
-	bool		missing_ok = false;
-	char	   *filename;
-	bytea	   *result;
-
-	/* handle optional arguments */
-	if (PG_NARGS() >= 3)
-	{
-		seek_offset = PG_GETARG_INT64(1);
-		bytes_to_read = PG_GETARG_INT64(2);
-
-		if (bytes_to_read < 0)
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("requested length cannot be negative")));
-	}
-	if (PG_NARGS() >= 4)
-		missing_ok = PG_GETARG_BOOL(3);
-
-	filename = convert_and_check_filename(filename_t);
-
-	result = read_binary_file(filename, seek_offset,
-							  bytes_to_read, missing_ok);
-	if (result)
-		PG_RETURN_BYTEA_P(result);
-	else
-		PG_RETURN_NULL();
+	if (read_to_eof)
+		Assert(bytes_to_read == -1);
+	else if (bytes_to_read < 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("requested length cannot be negative")));
+	
+	return read_binary_file(convert_and_check_filename(filename_t),
+							seek_offset, bytes_to_read, missing_ok);
 }
 
 
 /*
- * Wrapper functions for the 1 and 3 argument variants of pg_read_file_v2()
- * and pg_read_binary_file().
+ * Wrapper functions for the variants of SQL functions pg_read_file() and
+ * pg_read_binary_file().
  *
  * These are necessary to pass the sanity check in opr_sanity, which checks
  * that all built-in functions that share the implementing C function take
@@ -361,25 +329,126 @@ pg_read_binary_file(PG_FUNCTION_ARGS)
 Datum
 pg_read_file_off_len(PG_FUNCTION_ARGS)
 {
-	return pg_read_file_v2(fcinfo);
+	text   *filename_t = PG_GETARG_TEXT_PP(0);
+	int64	seek_offset = PG_GETARG_INT64(1);
+	int64	bytes_to_read = PG_GETARG_INT64(2);
+	text   *ret;
+
+	ret = pg_read_file_common(filename_t, seek_offset, bytes_to_read,
+							  false, false);
+	if (!ret)
+		PG_RETURN_NULL();
+
+	PG_RETURN_TEXT_P(ret);
+}
+
+Datum
+pg_read_file_off_len_missing(PG_FUNCTION_ARGS)
+{
+	text   *filename_t = PG_GETARG_TEXT_PP(0);
+	int64	seek_offset = PG_GETARG_INT64(1);
+	int64	bytes_to_read = PG_GETARG_INT64(2);
+	bool	missing_ok = PG_GETARG_BOOL(3);
+	text   *ret;
+
+	ret = pg_read_file_common(filename_t, seek_offset, bytes_to_read,
+							  false, missing_ok);
+	
+	if (!ret)
+		PG_RETURN_NULL();
+
+	PG_RETURN_TEXT_P(ret);
 }
 
 Datum
 pg_read_file_all(PG_FUNCTION_ARGS)
 {
-	return pg_read_file_v2(fcinfo);
+	text   *filename_t = PG_GETARG_TEXT_PP(0);
+	text   *ret;
+
+	ret = pg_read_file_common(filename_t, 0, -1, true, false);
+
+	if (!ret)
+		PG_RETURN_NULL();
+
+	PG_RETURN_TEXT_P(ret);
+}
+
+Datum
+pg_read_file_all_missing(PG_FUNCTION_ARGS)
+{
+	text   *filename_t = PG_GETARG_TEXT_PP(0);
+	bool	missing_ok = PG_GETARG_BOOL(1);
+	text   *ret;
+
+	ret = pg_read_file_common(filename_t, 0, -1, true, missing_ok);
+
+	if (!ret)
+		PG_RETURN_NULL();
+
+	PG_RETURN_TEXT_P(ret);
 }
 
 Datum
 pg_read_binary_file_off_len(PG_FUNCTION_ARGS)
 {
-	return pg_read_binary_file(fcinfo);
+	text   *filename_t = PG_GETARG_TEXT_PP(0);
+	int64	seek_offset = PG_GETARG_INT64(1);
+	int64	bytes_to_read = PG_GETARG_INT64(2);
+	text   *ret;
+
+	ret = pg_read_binary_file_common(filename_t, seek_offset, bytes_to_read,
+									 false, false);
+	if (!ret)
+		PG_RETURN_NULL();
+
+	PG_RETURN_BYTEA_P(ret);
+}
+
+Datum
+pg_read_binary_file_off_len_missing(PG_FUNCTION_ARGS)
+{
+	text   *filename_t = PG_GETARG_TEXT_PP(0);
+	int64	seek_offset = PG_GETARG_INT64(1);
+	int64	bytes_to_read = PG_GETARG_INT64(2);
+	bool	missing_ok = PG_GETARG_BOOL(3);
+	text   *ret;
+
+	ret = pg_read_binary_file_common(filename_t, seek_offset, bytes_to_read,
+									 false, missing_ok);
+	if (!ret)
+		PG_RETURN_NULL();
+
+	PG_RETURN_BYTEA_P(ret);
 }
 
 Datum
 pg_read_binary_file_all(PG_FUNCTION_ARGS)
 {
-	return pg_read_binary_file(fcinfo);
+	text   *filename_t = PG_GETARG_TEXT_PP(0);
+	text   *ret;
+
+	ret = pg_read_binary_file_common(filename_t, 0, -1, true, false);
+
+	if (!ret)
+		PG_RETURN_NULL();
+
+	PG_RETURN_BYTEA_P(ret);
+}
+
+Datum
+pg_read_binary_file_all_missing(PG_FUNCTION_ARGS)
+{
+	text   *filename_t = PG_GETARG_TEXT_PP(0);
+	bool	missing_ok = PG_GETARG_BOOL(1);
+	text   *ret;
+
+	ret = pg_read_binary_file_common(filename_t, 0, -1, true, missing_ok);
+
+	if (!ret)
+		PG_RETURN_NULL();
+
+	PG_RETURN_BYTEA_P(ret);
 }
 
 /*
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 2e41f4d9e8..63392d24ff 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6415,7 +6415,7 @@
   proargtypes => 'text int8 int8', prosrc => 'pg_read_file_off_len' },
 { oid => '3293', descr => 'read text from a file',
   proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
-  proargtypes => 'text int8 int8 bool', prosrc => 'pg_read_file_v2' },
+  proargtypes => 'text int8 int8 bool', prosrc => 'pg_read_file_off_len_missing' },
 { oid => '4100',
   descr => 'read text from a file - old version for adminpack 1.0',
   proname => 'pg_read_file_old', provolatile => 'v', prorettype => 'text',
@@ -6423,15 +6423,22 @@
 { oid => '3826', descr => 'read text from a file',
   proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
   proargtypes => 'text', prosrc => 'pg_read_file_all' },
+{ oid => '8025', descr => 'read text from a file',
+  proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
+  proargtypes => 'text bool', prosrc => 'pg_read_file_all_missing' },
 { oid => '3827', descr => 'read bytea from a file',
   proname => 'pg_read_binary_file', provolatile => 'v', prorettype => 'bytea',
   proargtypes => 'text int8 int8', prosrc => 'pg_read_binary_file_off_len' },
 { oid => '3295', descr => 'read bytea from a file',
   proname => 'pg_read_binary_file', provolatile => 'v', prorettype => 'bytea',
-  proargtypes => 'text int8 int8 bool', prosrc => 'pg_read_binary_file' },
+  proargtypes => 'text int8 int8 bool',
+  prosrc => 'pg_read_binary_file_off_len_missing' },
 { oid => '3828', descr => 'read bytea from a file',
   proname => 'pg_read_binary_file', provolatile => 'v', prorettype => 'bytea',
   proargtypes => 'text', prosrc => 'pg_read_binary_file_all' },
+{ oid => '8026', descr => 'read bytea from a file',
+  proname => 'pg_read_binary_file', provolatile => 'v', prorettype => 'bytea',
+  proargtypes => 'text bool', prosrc => 'pg_read_binary_file_all_missing' },
 { oid => '2625', descr => 'list all files in a directory',
   proname => 'pg_ls_dir', prorows => '1000', proretset => 't',
   provolatile => 'v', prorettype => 'text', proargtypes => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 01d1ad0b9a..07e9348f97 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -372,6 +372,83 @@ select count(*) >= 0 as ok from pg_ls_archive_statusdir();
  t
 (1 row)
 
+-- pg_read_file()
+\pset null <null>
+select substr(pg_read_file('pg_hba.conf'), 1, 30);
+             substr             
+--------------------------------
+ # PostgreSQL Client Authentica
+(1 row)
+
+select pg_read_file('pg_hba.conf', 0, 30);
+          pg_read_file          
+--------------------------------
+ # PostgreSQL Client Authentica
+(1 row)
+
+-- Test missing_ok
+select pg_read_file('does not exist'); -- error
+ERROR:  could not open file "does not exist" for reading: No such file or directory
+select pg_read_file('does not exist', false); -- error
+ERROR:  could not open file "does not exist" for reading: No such file or directory
+select pg_read_file('does not exist', 0, 30, false); -- error
+ERROR:  could not open file "does not exist" for reading: No such file or directory
+select pg_read_file('does not exist', true); -- ok
+ pg_read_file 
+--------------
+ <null>
+(1 row)
+
+select pg_read_file('does not exist', 0, 30, true); -- ok
+ pg_read_file 
+--------------
+ <null>
+(1 row)
+
+-- Test invalid argument
+select pg_read_file('does not exist', 0, -1); -- error
+ERROR:  requested length cannot be negative
+select pg_read_file('does not exist', 0, -1, true); -- error
+ERROR:  requested length cannot be negative
+-- pg_read_binary_file()
+select substr(pg_read_binary_file('pg_hba.conf'), 1, 30);
+                             substr                             
+----------------------------------------------------------------
+ \x2320506f737467726553514c20436c69656e742041757468656e74696361
+(1 row)
+
+select pg_read_binary_file('pg_hba.conf', 0, 30);
+                      pg_read_binary_file                       
+----------------------------------------------------------------
+ \x2320506f737467726553514c20436c69656e742041757468656e74696361
+(1 row)
+
+-- Test missing_ok
+select pg_read_binary_file('does not exist'); -- error
+ERROR:  could not open file "does not exist" for reading: No such file or directory
+select pg_read_binary_file('does not exist', false); -- error
+ERROR:  could not open file "does not exist" for reading: No such file or directory
+select pg_read_binary_file('does not exist', 0, 30, false); -- error
+ERROR:  could not open file "does not exist" for reading: No such file or directory
+select pg_read_binary_file('does not exist', true); -- ok
+ pg_read_binary_file 
+---------------------
+ <null>
+(1 row)
+
+select pg_read_binary_file('does not exist', 0, 30, true); -- ok
+ pg_read_binary_file 
+---------------------
+ <null>
+(1 row)
+
+-- Test invalid argument
+select pg_read_binary_file('does not exist', 0, -1); -- error
+ERROR:  requested length cannot be negative
+select pg_read_binary_file('does not exist', 0, -1, true); -- error
+ERROR:  requested length cannot be negative
+\pset null
+-- pg_ls_dir()
 select * from (select pg_ls_dir('.') a) a where a = 'base' limit 1;
   a   
 ------
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 072fc36a1f..8f5251abbb 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -123,6 +123,35 @@ from (select pg_ls_waldir() w) ss where length((w).name) = 24 limit 1;
 
 select count(*) >= 0 as ok from pg_ls_archive_statusdir();
 
+-- pg_read_file()
+\pset null <null>
+select substr(pg_read_file('pg_hba.conf'), 1, 30);
+select pg_read_file('pg_hba.conf', 0, 30);
+-- Test missing_ok
+select pg_read_file('does not exist'); -- error
+select pg_read_file('does not exist', false); -- error
+select pg_read_file('does not exist', 0, 30, false); -- error
+select pg_read_file('does not exist', true); -- ok
+select pg_read_file('does not exist', 0, 30, true); -- ok
+-- Test invalid argument
+select pg_read_file('does not exist', 0, -1); -- error
+select pg_read_file('does not exist', 0, -1, true); -- error
+
+-- pg_read_binary_file()
+select substr(pg_read_binary_file('pg_hba.conf'), 1, 30);
+select pg_read_binary_file('pg_hba.conf', 0, 30);
+-- Test missing_ok
+select pg_read_binary_file('does not exist'); -- error
+select pg_read_binary_file('does not exist', false); -- error
+select pg_read_binary_file('does not exist', 0, 30, false); -- error
+select pg_read_binary_file('does not exist', true); -- ok
+select pg_read_binary_file('does not exist', 0, 30, true); -- ok
+-- Test invalid argument
+select pg_read_binary_file('does not exist', 0, -1); -- error
+select pg_read_binary_file('does not exist', 0, -1, true); -- error
+\pset null
+
+-- pg_ls_dir()
 select * from (select pg_ls_dir('.') a) a where a = 'base' limit 1;
 -- Test missing_ok (second argument)
 select pg_ls_dir('does not exist', false, false); -- error
@@ -133,6 +162,8 @@ select count(*) = 1 as dot_found
 select count(*) = 1 as dot_found
   from pg_ls_dir('.', false, false) as ls where ls = '.';
 
+
+
 select * from (select (pg_timezone_names()).name) ptn where name='UTC' limit 1;
 
 select count(*) > 0 from
-- 
2.31.1

