Hi! On Fri, Jan 11, 2019 at 8:51 AM Andreas Karlsson <andr...@proxel.se> wrote: > Her is quick initial review. I will do more testing later.
Thanks for doing the review! > 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". > 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. [1] https://github.com/postgres/postgres/blob/master/src/backend/commands/view.c#L554 Mitar -- http://mitar.tnode.com/ https://twitter.com/mitar_m
