Arrange for the pg_foo_is_visible and has_foo_privilege families of functions

to return NULL, instead of erroring out, if the target object is specified by
OID and we can't find that OID in the catalogs.  Since these functions operate
internally on SnapshotNow rules, there is a race condition when using them
in user queries: the query's MVCC snapshot might "see" a catalog row that's
already committed dead, leading to a failure when the inquiry function is
applied.  Returning NULL should generally provide more convenient behavior.
This issue has been complained of before, and in particular we are now seeing
it in the regression tests due to another recent patch.

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 6c83755a4c7d999eff31d75f520a050e5ba9c32e..e665e9594d95b8446e8235d9a358d7732f3ff8cc 100644 (file)
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.113 2008/12/04 17:51:26 petere Exp $
+ *       $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.114 2008/12/15 18:09:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3250,6 +3250,15 @@ fetch_search_path_array(Oid *sarray, int sarray_len)
 
 /*
  * Export the FooIsVisible functions as SQL-callable functions.
+ *
+ * Note: as of Postgres 8.4, these will silently return NULL if called on
+ * a nonexistent object OID, rather than failing.  This is to avoid race
+ * condition errors when a query that's scanning a catalog using an MVCC
+ * snapshot uses one of these functions.  The underlying IsVisible functions
+ * operate on SnapshotNow semantics and so might see the object as already
+ * gone when it's still visible to the MVCC snapshot.  (There is no race
+ * condition in the current coding because we don't accept sinval messages
+ * between the SearchSysCacheExists test and the subsequent lookup.)
  */
 
 Datum
@@ -3257,6 +3266,11 @@ pg_table_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(RELOID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(RelationIsVisible(oid));
 }
 
@@ -3265,6 +3279,11 @@ pg_type_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(TYPEOID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(TypeIsVisible(oid));
 }
 
@@ -3273,6 +3292,11 @@ pg_function_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(PROCOID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(FunctionIsVisible(oid));
 }
 
@@ -3281,6 +3305,11 @@ pg_operator_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(OPEROID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(OperatorIsVisible(oid));
 }
 
@@ -3289,6 +3318,11 @@ pg_opclass_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(CLAOID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(OpclassIsVisible(oid));
 }
 
@@ -3297,6 +3331,11 @@ pg_conversion_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(CONVOID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(ConversionIsVisible(oid));
 }
 
@@ -3305,6 +3344,11 @@ pg_ts_parser_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(TSPARSEROID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(TSParserIsVisible(oid));
 }
 
@@ -3313,6 +3357,11 @@ pg_ts_dict_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(TSDICTOID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(TSDictionaryIsVisible(oid));
 }
 
@@ -3321,6 +3370,11 @@ pg_ts_template_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(TSTEMPLATEOID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(TSTemplateIsVisible(oid));
 }
 
@@ -3329,6 +3383,11 @@ pg_ts_config_is_visible(PG_FUNCTION_ARGS)
 {
        Oid                     oid = PG_GETARG_OID(0);
 
+       if (!SearchSysCacheExists(TSCONFIGOID,
+                                                         ObjectIdGetDatum(oid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        PG_RETURN_BOOL(TSConfigIsVisible(oid));
 }
 

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 16d5f64fb091d7a7b664b1903ef48a04c2cbe8ca..963112ef74a369cb388d0b4388bc0472b6d47d7d 100644 (file)
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.142 2008/09/09 18:58:08 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.143 2008/12/15 18:09:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1361,7 +1361,9 @@ convert_priv_string(text *priv_type_text)
  *             user name, user OID, or implicit user = current_user.
  *
  *             The result is a boolean value: true if user has the indicated
- *             privilege, false if not.
+ *             privilege, false if not.  The variants that take a relation OID
+ *             return NULL if the OID doesn't exist (rather than failing, as
+ *             they did before Postgres 8.4).
  */
 
 /*
@@ -1432,6 +1434,11 @@ has_table_privilege_name_id(PG_FUNCTION_ARGS)
        roleid = get_roleid_checked(NameStr(*username));
        mode = convert_table_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(RELOID,
+                                                         ObjectIdGetDatum(tableoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_class_aclcheck(tableoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -1455,6 +1462,11 @@ has_table_privilege_id(PG_FUNCTION_ARGS)
        roleid = GetUserId();
        mode = convert_table_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(RELOID,
+                                                         ObjectIdGetDatum(tableoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_class_aclcheck(tableoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -1499,6 +1511,11 @@ has_table_privilege_id_id(PG_FUNCTION_ARGS)
 
        mode = convert_table_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(RELOID,
+                                                         ObjectIdGetDatum(tableoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_class_aclcheck(tableoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -1587,7 +1604,7 @@ convert_table_priv_string(text *priv_type_text)
  *             user name, user OID, or implicit user = current_user.
  *
  *             The result is a boolean value: true if user has the indicated
- *             privilege, false if not.
+ *             privilege, false if not, or NULL if object doesn't exist.
  */
 
 /*
@@ -1658,6 +1675,11 @@ has_database_privilege_name_id(PG_FUNCTION_ARGS)
        roleid = get_roleid_checked(NameStr(*username));
        mode = convert_database_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(DATABASEOID,
+                                                         ObjectIdGetDatum(databaseoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_database_aclcheck(databaseoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -1681,6 +1703,11 @@ has_database_privilege_id(PG_FUNCTION_ARGS)
        roleid = GetUserId();
        mode = convert_database_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(DATABASEOID,
+                                                         ObjectIdGetDatum(databaseoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_database_aclcheck(databaseoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -1725,6 +1752,11 @@ has_database_privilege_id_id(PG_FUNCTION_ARGS)
 
        mode = convert_database_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(DATABASEOID,
+                                                         ObjectIdGetDatum(databaseoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_database_aclcheck(databaseoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -1798,7 +1830,7 @@ convert_database_priv_string(text *priv_type_text)
  *             user name, user OID, or implicit user = current_user.
  *
  *             The result is a boolean value: true if user has the indicated
- *             privilege, false if not.
+ *             privilege, false if not, or NULL if object doesn't exist.
  */
 
 /*
@@ -1869,6 +1901,11 @@ has_function_privilege_name_id(PG_FUNCTION_ARGS)
        roleid = get_roleid_checked(NameStr(*username));
        mode = convert_function_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(PROCOID,
+                                                         ObjectIdGetDatum(functionoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_proc_aclcheck(functionoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -1892,6 +1929,11 @@ has_function_privilege_id(PG_FUNCTION_ARGS)
        roleid = GetUserId();
        mode = convert_function_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(PROCOID,
+                                                         ObjectIdGetDatum(functionoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_proc_aclcheck(functionoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -1936,6 +1978,11 @@ has_function_privilege_id_id(PG_FUNCTION_ARGS)
 
        mode = convert_function_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(PROCOID,
+                                                         ObjectIdGetDatum(functionoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_proc_aclcheck(functionoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -1996,7 +2043,7 @@ convert_function_priv_string(text *priv_type_text)
  *             user name, user OID, or implicit user = current_user.
  *
  *             The result is a boolean value: true if user has the indicated
- *             privilege, false if not.
+ *             privilege, false if not, or NULL if object doesn't exist.
  */
 
 /*
@@ -2067,6 +2114,11 @@ has_language_privilege_name_id(PG_FUNCTION_ARGS)
        roleid = get_roleid_checked(NameStr(*username));
        mode = convert_language_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(LANGOID,
+                                                         ObjectIdGetDatum(languageoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_language_aclcheck(languageoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -2090,6 +2142,11 @@ has_language_privilege_id(PG_FUNCTION_ARGS)
        roleid = GetUserId();
        mode = convert_language_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(LANGOID,
+                                                         ObjectIdGetDatum(languageoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_language_aclcheck(languageoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -2134,6 +2191,11 @@ has_language_privilege_id_id(PG_FUNCTION_ARGS)
 
        mode = convert_language_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(LANGOID,
+                                                         ObjectIdGetDatum(languageoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_language_aclcheck(languageoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -2194,7 +2256,7 @@ convert_language_priv_string(text *priv_type_text)
  *             user name, user OID, or implicit user = current_user.
  *
  *             The result is a boolean value: true if user has the indicated
- *             privilege, false if not.
+ *             privilege, false if not, or NULL if object doesn't exist.
  */
 
 /*
@@ -2265,6 +2327,11 @@ has_schema_privilege_name_id(PG_FUNCTION_ARGS)
        roleid = get_roleid_checked(NameStr(*username));
        mode = convert_schema_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(NAMESPACEOID,
+                                                         ObjectIdGetDatum(schemaoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_namespace_aclcheck(schemaoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -2288,6 +2355,11 @@ has_schema_privilege_id(PG_FUNCTION_ARGS)
        roleid = GetUserId();
        mode = convert_schema_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(NAMESPACEOID,
+                                                         ObjectIdGetDatum(schemaoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_namespace_aclcheck(schemaoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
@@ -2332,6 +2404,11 @@ has_schema_privilege_id_id(PG_FUNCTION_ARGS)
 
        mode = convert_schema_priv_string(priv_type_text);
 
+       if (!SearchSysCacheExists(NAMESPACEOID,
+                                                         ObjectIdGetDatum(schemaoid),
+                                                         0, 0, 0))
+               PG_RETURN_NULL();
+
        aclresult = pg_namespace_aclcheck(schemaoid, roleid, mode);
 
        PG_RETURN_BOOL(aclresult == ACLCHECK_OK);

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index d1767e95ad4cdab702466aaf5e18e3d5842246d4..607c591c0e6f7122425918ad44f3e355149f45cc 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -321,7 +321,11 @@ ERROR:  unrecognized privilege type: "sel"
 select has_table_privilege(-999999,'pg_authid','update');
 ERROR:  role with OID 4293967297 does not exist
 select has_table_privilege(1,'select');
-ERROR:  relation with OID 1 does not exist
+ has_table_privilege 
+---------------------
+ 
+(1 row)
+
 -- superuser
 \c -
 select has_table_privilege(current_user,'pg_authid','select');