Re: Proposal to suppress errors thrown by to_reg*()

From: Takuma Hoshiai <hoshiai(at)sraoss(dot)co(dot)jp>
To: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Proposal to suppress errors thrown by to_reg*()
Date: 2019-03-22 00:45:09
Message-ID: 20190322094509.7fbd7e0d3bfa32086b0a43a8@sraoss.co.jp
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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(at)sraoss(dot)co(dot)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(at)sraoss(dot)co(dot)jp>

Attachment Content-Type Size
fix_to_reg_v2.patch application/octet-stream 7.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-03-22 01:23:54 Re: current_logfiles not following group access and instead follows log_file_mode permissions
Previous Message Michael Paquier 2019-03-22 00:42:36 Re: pg_basebackup ignores the existing data directory permissions