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
