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 <nagata@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 <hoshiai@sraoss.co.jp>