Stephen, so far I've read thru your patch and familiarized myself with some of 
the auth functionality in pg_authid.h and src/backend/utils/adt/acl.c

The only question I have so far about your patch is the last several hunks of 
the diff, which remove superuser checks without adding anything immediately 
obvious in their place:

...
@@ -195,11 +205,6 @@ pg_read_file(PG_FUNCTION_ARGS)
    char       *filename;
    text       *result;
 
-   if (!superuser())
-       ereport(ERROR,
-               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-                (errmsg("must be superuser to read files"))));
-
    /* handle optional arguments */
    if (PG_NARGS() >= 3)
    {
@@ -236,11 +241,6 @@ pg_read_binary_file(PG_FUNCTION_ARGS)
    char       *filename;
    bytea      *result;
 
-   if (!superuser())
-       ereport(ERROR,
-               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-                (errmsg("must be superuser to read files"))));
-
    /* handle optional arguments */
    if (PG_NARGS() >= 3)
    {
@@ -313,11 +313,6 @@ pg_stat_file(PG_FUNCTION_ARGS)
    TupleDesc   tupdesc;
    bool        missing_ok = false;
 
-   if (!superuser())
-       ereport(ERROR,
-               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-                (errmsg("must be superuser to get file information"))));
-
    /* check the optional argument */
    if (PG_NARGS() == 2)
        missing_ok = PG_GETARG_BOOL(1);
...

I wanted to ask if you have reason to believe that these checks were not 
necessary (and therefore can be deleted instead of replaced by 
is_member_of_role() checks like you did elsewhere).  I still have limited 
understanding of the overall code, so really just asking because it's the first 
thing that jumped out.

Best,
Ryan

Reply via email to