Avoid reporting "cache lookup failed" for some user-reachable cases.

We have a not-terribly-thoroughly-enforced-yet project policy that internal
errors with SQLSTATE XX000 (ie, plain elog) should not be triggerable from
SQL.  record_in, domain_in, and PL validator functions all failed to meet
this standard, because they threw plain elog("cache lookup failed for XXX")
errors on bad OIDs, and those are all invokable from SQL.

For record_in, the best fix is to upgrade typcache.c (lookup_type_cache)
to throw a user-facing error for this case.  That seems consistent because
it was more than halfway there already, having user-facing errors for shell
types and non-composite types.  Having done that, tweak domain_in to rely
on the typcache to throw an appropriate error.  (This costs little because
InitDomainConstraintRef would fetch the typcache entry anyway.)

For the PL validator functions, we already have a single choke point at
CheckFunctionValidatorAccess, so just fix its error to be user-facing.

Dilip Kumar, reviewed by Haribabu Kommi

Discussion: <87wpxfygg9.fsf@credativ.de>

diff --git a/src/backend/utils/adt/domains.c b/src/backend/utils/adt/domains.c
index 19ee4ce9d164d013db7d6f0162ed19e10dd68679..26bbbb597974b4bf0fd9040afef34e1a9bb6066a 100644 (file)
--- a/src/backend/utils/adt/domains.c
+++ b/src/backend/utils/adt/domains.c
@@ -72,19 +72,28 @@ static DomainIOData *
 domain_state_setup(Oid domainType, bool binary, MemoryContext mcxt)
 {
        DomainIOData *my_extra;
+       TypeCacheEntry *typentry;
        Oid                     baseType;
 
        my_extra = (DomainIOData *) MemoryContextAlloc(mcxt, sizeof(DomainIOData));
 
-       /* Find out the base type */
-       my_extra->typtypmod = -1;
-       baseType = getBaseTypeAndTypmod(domainType, &my_extra->typtypmod);
-       if (baseType == domainType)
+       /*
+        * Verify that domainType represents a valid domain type.  We need to be
+        * careful here because domain_in and domain_recv can be called from SQL,
+        * possibly with incorrect arguments.  We use lookup_type_cache mainly
+        * because it will throw a clean user-facing error for a bad OID.
+        */
+       typentry = lookup_type_cache(domainType, 0);
+       if (typentry->typtype != TYPTYPE_DOMAIN)
                ereport(ERROR,
                                (errcode(ERRCODE_DATATYPE_MISMATCH),
                                 errmsg("type %s is not a domain",
                                                format_type_be(domainType))));
 
+       /* Find out the base type */
+       my_extra->typtypmod = -1;
+       baseType = getBaseTypeAndTypmod(domainType, &my_extra->typtypmod);
+
        /* Look up underlying I/O function */
        if (binary)
                getTypeBinaryInputInfo(baseType,

diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 9150fe832f59235fc4f35e738a00884de431ddd8..2d0db764e344cffd18f0d8c8bcb2d3c76cee3a20 100644 (file)
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -182,10 +182,10 @@ static int        enum_oid_cmp(const void *left, const void *right);
  * Fetch the type cache entry for the specified datatype, and make sure that
  * all the fields requested by bits in 'flags' are valid.
  *
- * The result is never NULL --- we will elog() if the passed type OID is
+ * The result is never NULL --- we will ereport() if the passed type OID is
  * invalid.  Note however that we may fail to find one or more of the
- * requested opclass-dependent fields; the caller needs to check whether
- * the fields are InvalidOid or not.
+ * values requested by 'flags'; the caller needs to check whether the fields
+ * are InvalidOid or not.
  */
 TypeCacheEntry *
 lookup_type_cache(Oid type_id, int flags)
@@ -224,14 +224,18 @@ lookup_type_cache(Oid type_id, int flags)
                /*
                 * If we didn't find one, we want to make one.  But first look up the
                 * pg_type row, just to make sure we don't make a cache entry for an
-                * invalid type OID.
+                * invalid type OID.  If the type OID is not valid, present a
+                * user-facing error, since some code paths such as domain_in() allow
+                * this function to be reached with a user-supplied OID.
                 */
                HeapTuple       tp;
                Form_pg_type typtup;
 
                tp = SearchSysCache1(TYPEOID, ObjectIdGetDatum(type_id));
                if (!HeapTupleIsValid(tp))
-                       elog(ERROR, "cache lookup failed for type %u", type_id);
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_UNDEFINED_OBJECT),
+                                        errmsg("type with OID %u does not exist", type_id)));
                typtup = (Form_pg_type) GETSTRUCT(tp);
                if (!typtup->typisdefined)
                        ereport(ERROR,
@@ -1230,6 +1234,8 @@ lookup_rowtype_tupdesc_internal(Oid type_id, int32 typmod, bool noError)
  *
  * Given a typeid/typmod that should describe a known composite type,
  * return the tuple descriptor for the type.  Will ereport on failure.
+ * (Use ereport because this is reachable with user-specified OIDs,
+ * for example from record_in().)
  *
  * Note: on success, we increment the refcount of the returned TupleDesc,
  * and log the reference in CurrentResourceOwner.  Caller should call

diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 7aae35074fa317c322bedf245e3d243994b8183b..46a55ba7b953b4786dd2ba07b57019cc86031e33 100644 (file)
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -2455,10 +2455,15 @@ CheckFunctionValidatorAccess(Oid validatorOid, Oid functionOid)
        Form_pg_language langStruct;
        AclResult       aclresult;
 
-       /* Get the function's pg_proc entry */
+       /*
+        * Get the function's pg_proc entry.  Throw a user-facing error for bad
+        * OID, because validators can be called with user-specified OIDs.
+        */
        procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(functionOid));
        if (!HeapTupleIsValid(procTup))
-               elog(ERROR, "cache lookup failed for function %u", functionOid);
+               ereport(ERROR,
+                               (errcode(ERRCODE_UNDEFINED_FUNCTION),
+                                errmsg("function with OID %u does not exist", functionOid)));
        procStruct = (Form_pg_proc) GETSTRUCT(procTup);
 
        /*