I took a look at 0001. On 7/26/18, 12:24 AM, "Michael Paquier" <mich...@paquier.xyz> wrote: > 1) 0001 does the work for TRUNCATE, but using RangeVarGetRelidExtended > with a custom callback based on the existing truncate_check_rel(). I > had to split the session-level checks into a separate routine as no > Relation is available, but that finishes by being a neat result without > changing any user-facing behavior.
Splitting the checks like this seems reasonable. As you pointed out, it doesn't change the behavior of the session checks, which AFAICT aren't necessary for the kind of permissions checks we want to add to the RangeVarGetRelidExtended() call. - myrelid = RelationGetRelid(rel); + myrelid = RangeVarGetRelidExtended(rv, AccessExclusiveLock, + false, RangeVarCallbackForTruncate, + NULL); Should the flags argument be 0 instead of false? + /* Nothing to do if the relation was not found. */ + if (!OidIsValid(relId)) + return; + + tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relId)); + if (!HeapTupleIsValid(tuple)) /* should not happen */ + elog(ERROR, "cache lookup failed for relation %u", relId); The first time we use this callback, the relation won't be locked, so isn't it possible that we won't get a valid tuple here? I did notice that callbacks like RangeVarCallbackForRenameRule, RangeVarCallbackForPolicy, and RangeVarCallbackForRenameTrigger assume that the relation can be concurrently dropped, but RangeVarCallbackOwnsRelation does not. Instead, we assume that the syscache search will succeed if the given OID is valid. Is this a bug, or am I missing something? Nathan
