Hi Nagata-san, sorry for te late reply. Thank you for your comments, I have attached a patch that reflected it.
On Tue, 19 Mar 2019 15:13:04 +0900 Yugo Nagata <nag...@sraoss.co.jp> wrote: > I reviewed the patch. Here are some comments: > > /* > + * RangeVarCheckNamespaceAccessNoError > + * Returns true if given relation's namespace can be accessable by > the > + * current user. If no namespace is given in the relation, just > returns > + * true. > + */ > +bool > +RangeVarCheckNamespaceAccessNoError(RangeVar *relation) > > Although it might be trivial, the new function's name 'RangeVar...' seems a > bit > weird to me because this is used for not only to_regclass but also to_regproc, > to_regtype, and so on, that is, the argument "relation" is not always a > relation. > > This function is used always with makeRangeVarFromNameList() as > > if (!RangeVarCheckNamespaceAccessNoError(makeRangeVarFromNameList(names))) > > , so how about merging the two function as below, for example: > > /* > * CheckNamespaceAccessNoError > * Returns true if the namespace in given qualified-name can be > accessable > * by the current user. If no namespace is given in the names, just > returns > * true. > */ > bool > CheckNamespaceAccessNoError(List *names); > > > BTW, this patch supresses also "Cross-database references is not allowed" > error in > addition to the namespace ACL error. Is this an intentional change? If this > error > can be allowed, you can use DeconstructQualifiedName() instead of > makeRangeVarFromNameList(). I think it is enough to supress napesapace ACL error only. so I will use its function. > In the regression test, you are using \gset and \connect psql meta-commands > to test > the user privilege to a namespace, but I think we can make this more simpler > by using SET SESSION AUTHORIZATION and RESET AUTHORIZATION. I forgot this SQL, I fixed to use it. Best regards, -- Takuma Hoshiai <hosh...@sraoss.co.jp>
fix_to_reg_v2.patch
Description: Binary data
