On Wed, Sep 07, 2022 at 08:48:43AM -0700, Jacob Champion wrote: > Also, this function's placement in the docs (with the System Catalog > Information Functions) seems wrong to me. I think it should go up above > in the Session Information Functions, with current_user et al.
Yeah, this had better use a <link>. >> + /* Initialize SystemUser now that MyClientConnectionInfo is >> restored. */ >> + InitializeSystemUser(MyClientConnectionInfo.authn_id, >> + >> hba_authname(MyClientConnectionInfo.auth_method)); > > It makes me a little nervous to call hba_authname(auth_method) without > checking to see that auth_method is actually valid (which is only true > if authn_id is not NULL). You have mentioned that a couple of months ago if I recall correctly, and we pass down an enum value. > We could pass the bare auth_method index, or update the documentation > for auth_method to state that it's guaranteed to be zero if authn_id is > NULL (and then enforce that). > > > case SVFOP_CURRENT_USER: > > case SVFOP_USER: > > case SVFOP_SESSION_USER: > > + case SVFOP_SYSTEM_USER: > > case SVFOP_CURRENT_CATALOG: > > case SVFOP_CURRENT_SCHEMA: > > svf->type = NAMEOID; > > Should this be moved to use TEXTOID instead? Yeah, it should. There is actually a second and much deeper issue here, in the shape of a collation problem. See the assertion failure in exprSetCollation(), because we expect SQLValueFunction nodes to return a name or a non-collatable type. However, for this case, we'd require a text to get rid of the 63-character limit, and that's a collatable type. This reminds me of the recent thread to work on getting rid of this limit for the name type.. -- Michael
signature.asc
Description: PGP signature
