On 1/11/19 8:47 PM, Mitar wrote:
>> In create_ctas_internal() why do you copy the relation even when you do
>> not modify it?
>
> I was modelling this after code in view.c [1]. I can move copy into the "if".
Makes sense.
>> Is it really ok to just remove SECURITY_RESTRICTED_OPERATION from
>> ExecCreateTableAs()? I feel it is there for a good reason and that we
>> preferably want to reduce the duration of SECURITY_RESTRICTED_OPERATION
>> to only include when we actually execute the query.
>
> The comment there said that this is not really necessary for security:
>
> "This is not necessary for security, but this keeps the behavior
> similar to REFRESH MATERIALIZED VIEW. Otherwise, one could create a
> materialized view not possible to refresh."
>
> Based on my experimentation, this is required to be able to use
> temporary materialized views, but it does mean one has to pay
> attention from where one can refresh. For example, you cannot refresh
> from outside of the current session, because temporary object is not
> available there. I have not seen any other example where refresh would
> not be possible.
>
> This is why I felt comfortable removing this. Also, no test failed
> after removing this.
Hm, I am still not convinced just removing it is a good idea. Sure, it
is not a security issue but usability is also important. The question is
how much this worsens usability and how much extra work it would be to
keep the restriction.
Btw, if we are going to remove SECURITY_RESTRICTED_OPERATION we should
remove more code. There is no reason to save and reset the bitmask if we
do not alter it.
Andreas