On Fri, 2024-10-18 at 07:47 +0200, Pavel Stehule wrote: > Without deeper checks I don't like using GetUserNameFromId for checking the validity of a role. > > Maybe it is better to use own read of syscache or wrap SetUserIdAndSecContext to do this check.
I agree; it was just the simplest way I could make it happen. It is ugly to allocate and return the user name, since we don't really need it.
I understand
I could write a dedicated function to check the existence of a user.
+1
> The comment > > + /* > + * The role could have been dropped since the trigger was queued. > + * In that case, give up and error out. > + */ > > doesn't explain well why the role can be dropped and why dependency doesn't protect against it.
The trigger queue exists only in memory, and PostgreSQL tracks dependencies only between persisted objects. Do you think that I should add a sentence like that to the comment?
yes, please. I think so it is not too intuitive. Inside a context of this patch it is ok, but without knowledge of this context, can be strange, why some role used for trigger can be invalid, although the transaction is not fully finished yet.
