Support pg_read_[binary_]file (filename, missing_ok).
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 29 Jul 2022 19:38:49 +0000 (15:38 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 29 Jul 2022 19:38:49 +0000 (15:38 -0400)
There wasn't an especially nice way to read all of a file while
passing missing_ok = true.  Add an additional overloaded variant
to support that use-case.

While here, refactor the C code to avoid a rats-nest of PG_NARGS
checks, instead handling the argument collection in the outer
wrapper functions.  It's a bit longer this way, but far more
straightforward.

(Upon looking at the code coverage report for genfile.c, I was
impelled to also add a test case for pg_stat_file() -- tgl)

Kyotaro Horiguchi

Discussion: https://postgr.es/m/20220607.160520.1984541900138970018.horikyota.ntt@gmail.com

doc/src/sgml/func.sgml
src/backend/catalog/system_functions.sql
src/backend/utils/adt/genfile.c
src/include/catalog/catversion.h
src/include/catalog/pg_proc.dat
src/test/regress/expected/misc_functions.out
src/test/regress/sql/misc_functions.sql

index 21f8ab73e2851ad52065da62167a70122af745b3..053d4dc650b6c215ccd1bf4a74631938b4fb96e5 100644 (file)
@@ -28411,13 +28411,23 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
     considered.
    </para>
 
+   <para>
+    When granting privilege on these functions, note that the table entries
+    showing optional parameters are mostly implemented as several physical
+    functions with different parameter lists.  Privilege must be granted
+    separately on each such function, if it is to be
+    used.  <application>psql</application>'s <command>\df</command> command
+    can be useful to check what the actual function signatures are.
+   </para>
+
    <para>
     Some of these functions take an optional <parameter>missing_ok</parameter>
     parameter, which specifies the behavior when the file or directory does
     not exist.  If <literal>true</literal>, the function
     returns <literal>NULL</literal> or an empty result set, as appropriate.
-    If <literal>false</literal>, an error is raised.  The default
-    is <literal>false</literal>.
+    If <literal>false</literal>, an error is raised.  (Failure conditions
+    other than <quote>file not found</quote> are reported as errors in any
+    case.)  The default is <literal>false</literal>.
    </para>
 
    <table id="functions-admin-genfile-table">
@@ -28636,7 +28646,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 +28671,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>
index 73da687d5dd2f936417c4ffc599502fcc05f6eda..30a048f6b09fa0f77525eebde8ec0ea25a2431c9 100644 (file)
@@ -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;
index 2bf52192567a37625fd613070d540e38c3750a69..2f1e907a10d87b2a0a34d8414d05f9dca864a758 100644 (file)
@@ -278,81 +278,50 @@ 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);
+   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")));
 
-   result = read_text_file(filename, seek_offset, bytes_to_read, missing_ok);
-   if (result)
-       PG_RETURN_TEXT_P(result);
-   else
-       PG_RETURN_NULL();
+   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
+ *
+ * Parameters are interpreted the same as 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);
+   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")));
 
-   result = read_binary_file(filename, seek_offset,
-                             bytes_to_read, missing_ok);
-   if (result)
-       PG_RETURN_BYTEA_P(result);
-   else
-       PG_RETURN_NULL();
+   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 +330,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);
 }
 
 /*
index 988f6e40033d4b8f5d3e5427becbe9b989cc8342..9c254dafa783ea2b25b2cedfef9864b2c239d138 100644 (file)
@@ -57,6 +57,6 @@
  */
 
 /*                         yyyymmddN */
-#define CATALOG_VERSION_NO 202207271
+#define CATALOG_VERSION_NO 202207291
 
 #endif
index 2e41f4d9e812f698653221816c9a9d34750cf1bc..be47583122b64ad17ec2572cbaf66c2e8328c21f 100644 (file)
   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',
 { 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',
index 01d1ad0b9a4de45cc14ed63869594a4293dc5dca..ca82d91f1af857dd976cdd18717b8ac9ac4b0f48 100644 (file)
@@ -372,6 +372,68 @@ select count(*) >= 0 as ok from pg_ls_archive_statusdir();
  t
 (1 row)
 
+-- pg_read_file()
+select length(pg_read_file('postgresql.auto.conf')) > 30;
+ ?column? 
+----------
+ t
+(1 row)
+
+select length(pg_read_file('postgresql.auto.conf', 1, 30));
+ length 
+--------
+     30
+(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', true) IS NULL; -- ok
+ ?column? 
+----------
+ t
+(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 length(pg_read_binary_file('postgresql.auto.conf')) > 30;
+ ?column? 
+----------
+ t
+(1 row)
+
+select length(pg_read_binary_file('postgresql.auto.conf', 1, 30));
+ length 
+--------
+     30
+(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', true) IS NULL; -- ok
+ ?column? 
+----------
+ t
+(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
+-- pg_stat_file()
+select size > 30, isdir from pg_stat_file('postgresql.auto.conf');
+ ?column? | isdir 
+----------+-------
+ t        | f
+(1 row)
+
+-- pg_ls_dir()
 select * from (select pg_ls_dir('.') a) a where a = 'base' limit 1;
   a   
 ------
@@ -401,12 +463,14 @@ select count(*) = 1 as dot_found
  f
 (1 row)
 
+-- pg_timezone_names()
 select * from (select (pg_timezone_names()).name) ptn where name='UTC' limit 1;
  name 
 ------
  UTC
 (1 row)
 
+-- pg_tablespace_databases()
 select count(*) > 0 from
   (select pg_tablespace_databases(oid) as pts from pg_tablespace
    where spcname = 'pg_default') pts
index 072fc36a1ffd0389695fa366098d34f7670a5eac..30212103cafda834b67c42e0618aa756514170af 100644 (file)
@@ -123,6 +123,30 @@ 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()
+select length(pg_read_file('postgresql.auto.conf')) > 30;
+select length(pg_read_file('postgresql.auto.conf', 1, 30));
+-- Test missing_ok
+select pg_read_file('does not exist'); -- error
+select pg_read_file('does not exist', true) IS NULL; -- 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 length(pg_read_binary_file('postgresql.auto.conf')) > 30;
+select length(pg_read_binary_file('postgresql.auto.conf', 1, 30));
+-- Test missing_ok
+select pg_read_binary_file('does not exist'); -- error
+select pg_read_binary_file('does not exist', true) IS NULL; -- 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
+
+-- pg_stat_file()
+select size > 30, isdir from pg_stat_file('postgresql.auto.conf');
+
+-- 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,8 +157,10 @@ select count(*) = 1 as dot_found
 select count(*) = 1 as dot_found
   from pg_ls_dir('.', false, false) as ls where ls = '.';
 
+-- pg_timezone_names()
 select * from (select (pg_timezone_names()).name) ptn where name='UTC' limit 1;
 
+-- pg_tablespace_databases()
 select count(*) > 0 from
   (select pg_tablespace_databases(oid) as pts from pg_tablespace
    where spcname = 'pg_default') pts