On Wed, Feb 17, 2021 at 8:04 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Wed, Feb 17, 2021 at 03:08:36PM +1300, Thomas Munro wrote:
> > tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(oid));
> > if (!HeapTupleIsValid(tp))
> > + {
> > + if (found)
> > + {
> > + *found = false;
> > + return NULL;
> > + }
> > elog(ERROR, "cache lookup failed for collation %u", oid);
> > + }
> > collform = (Form_pg_collation) GETSTRUCT(tp);
> > version = get_collation_actual_version(collform->collprovider,
> >
NameStr(collform->collcollate));
> > + if (found)
> > + *found = true;
> > }
>
> FWIW, we usually prefer using NULL instead of an error for the result
> of a system function if an object cannot be found because it allows
> users to not get failures in a middle of a full table scan if things
> like an InvalidOid is mixed in the data set. For example, we do that
> in the partition functions, for objectaddress functions, etc. That
> would make this patch set simpler, switching
> get_collation_version_for_oid() to just use a missing_ok argument.
> And that would be more consistent with the other syscache lookup
> functions we have here and there in the tree.
I guess I was trying to preserve a distinction between "unknown OID"
and "this is a collation OID, but I don't have version information for
it" (for example, "C.utf8"). But it hardly matters, and your
suggestion works for me. Thanks for looking!