Feature: temporary materialized views

classic Classic list List threaded Threaded
40 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Feature: temporary materialized views

Mitar
Hi!

Sometimes materialized views are used to cache a complex query on
which a client works. But after client disconnects, the materialized
view could be deleted. Regular VIEWs and TABLEs both have support for
temporary versions which get automatically dropped at the end of the
session. It seems it is easy to add the same thing for materialized
views as well. See attached PoC patch.


Mitar

--
http://mitar.tnode.com/
https://twitter.com/mitar_m

tempmatviews.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Alvaro Herrera-9
On 2018-Dec-25, Mitar wrote:

> Sometimes materialized views are used to cache a complex query on
> which a client works. But after client disconnects, the materialized
> view could be deleted. Regular VIEWs and TABLEs both have support for
> temporary versions which get automatically dropped at the end of the
> session. It seems it is easy to add the same thing for materialized
> views as well. See attached PoC patch.

I think MVs that are dropped at session end are a sensible feature.  I
probably wouldn't go as far as allowing ON COMMIT actions, though, so
this much effort is the right amount.

I think if you really want to do this you should just use OptTemp, and
delete OptNoLog.  Of course, you need to add tests and patch the docs.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Mitar
Hi!

On Wed, Dec 26, 2018 at 9:00 AM Alvaro Herrera <[hidden email]> wrote:
> I think MVs that are dropped at session end are a sensible feature.

Thanks.

> I probably wouldn't go as far as allowing ON COMMIT actions, though

I agree. I do not see much usefulness for it. The only use case I can
think of would be to support REFRESH as an ON COMMIT action. That
would be maybe useful in the MV setting. After every transaction in my
session, REFRESH this materialized view.

But personally I do not have an use case for that, so I will leave it
to somebody else. :-)

> I think if you really want to do this you should just use OptTemp, and
> delete OptNoLog.

Sounds good.

OptTemp seems to have a misleading warning in some cases when it is
not used on tables though:

"GLOBAL is deprecated in temporary table creation"

Should we change this language to something else? "GLOBAL is
deprecated in temporary object creation"? Based on grammar it seems to
be used for tables, views, sequences, and soon materialized views.

> Of course, you need to add tests and patch the docs.

Sure.

[1] https://www.postgresql.org/message-id/29165.1545842105%40sss.pgh.pa.us


Mitar

--
http://mitar.tnode.com/
https://twitter.com/mitar_m

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Pavel Stehule


st 26. 12. 2018 v 18:20 odesílatel Mitar <[hidden email]> napsal:
Hi!

On Wed, Dec 26, 2018 at 9:00 AM Alvaro Herrera <[hidden email]> wrote:
> I think MVs that are dropped at session end are a sensible feature.

Thanks.

> I probably wouldn't go as far as allowing ON COMMIT actions, though

I agree. I do not see much usefulness for it. The only use case I can
think of would be to support REFRESH as an ON COMMIT action. That
would be maybe useful in the MV setting. After every transaction in my
session, REFRESH this materialized view.

But personally I do not have an use case for that, so I will leave it
to somebody else. :-)

> I think if you really want to do this you should just use OptTemp, and
> delete OptNoLog.

Sounds good.

OptTemp seems to have a misleading warning in some cases when it is
not used on tables though:

"GLOBAL is deprecated in temporary table creation"

Should we change this language to something else? "GLOBAL is
deprecated in temporary object creation"? Based on grammar it seems to
be used for tables, views, sequences, and soon materialized views.

This message is wrong - probably better "GLOBAL temporary tables are not supported"

Regards

Pavel

> Of course, you need to add tests and patch the docs.

Sure.

[1] https://www.postgresql.org/message-id/29165.1545842105%40sss.pgh.pa.us


Mitar

--
http://mitar.tnode.com/
https://twitter.com/mitar_m

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Alvaro Herrera-9
In reply to this post by Mitar
On 2018-Dec-26, Mitar wrote:

> OptTemp seems to have a misleading warning in some cases when it is
> not used on tables though:
>
> "GLOBAL is deprecated in temporary table creation"
>
> Should we change this language to something else? "GLOBAL is
> deprecated in temporary object creation"? Based on grammar it seems to
> be used for tables, views, sequences, and soon materialized views.

I'd just leave those messages alone.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Mitar
In reply to this post by Alvaro Herrera-9
Hi!

I made a new version of the patch. I added tests and changes to the
docs and made sure various other aspects of this change for as well. I
think this now makes temporary materialized views fully implemented
and that in my view patch is complete. If there is anything else to
add, please let me know, I do not yet have much experience
contributing here. What are next steps? Do I just wait for it to be
included into Commitfest? Do I add it there myself?


Mitar

On Wed, Dec 26, 2018 at 9:00 AM Alvaro Herrera <[hidden email]> wrote:

>
> On 2018-Dec-25, Mitar wrote:
>
> > Sometimes materialized views are used to cache a complex query on
> > which a client works. But after client disconnects, the materialized
> > view could be deleted. Regular VIEWs and TABLEs both have support for
> > temporary versions which get automatically dropped at the end of the
> > session. It seems it is easy to add the same thing for materialized
> > views as well. See attached PoC patch.
>
> I think MVs that are dropped at session end are a sensible feature.  I
> probably wouldn't go as far as allowing ON COMMIT actions, though, so
> this much effort is the right amount.
>
> I think if you really want to do this you should just use OptTemp, and
> delete OptNoLog.  Of course, you need to add tests and patch the docs.
>
> --
> Álvaro Herrera                https://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
http://mitar.tnode.com/
https://twitter.com/mitar_m

tempmatviews-v2.patch (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Alvaro Herrera-9
On 2018-Dec-27, Mitar wrote:

> Hi!
>
> I made a new version of the patch. I added tests and changes to the
> docs and made sure various other aspects of this change for as well. I
> think this now makes temporary materialized views fully implemented
> and that in my view patch is complete. If there is anything else to
> add, please let me know, I do not yet have much experience
> contributing here. What are next steps? Do I just wait for it to be
> included into Commitfest? Do I add it there myself?

Yes, please add it yourself to the commitfest.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Mitar
Hi!

Thanks, I did it.

I am attaching a new version of the patch with few more lines added to tests.

I noticed that there is no good summary of the latest patch, so let me
make it here:

So the latest version of the patch adds an option for "temporary"
materialized views. Such materialized views are automatically deleted
at the end of the session. Moreover, it also modifies the materialized
view creation logic so that now if any of the source relations are
temporary, the final materialized view is temporary as well. This now
makes materialized views more aligned with regular views.

Tests test that this really works, that refreshing of such views work,
and that refreshing can also work from a trigger.


Mitar

On Thu, Dec 27, 2018 at 5:15 AM Alvaro Herrera <[hidden email]> wrote:

>
> On 2018-Dec-27, Mitar wrote:
>
> > Hi!
> >
> > I made a new version of the patch. I added tests and changes to the
> > docs and made sure various other aspects of this change for as well. I
> > think this now makes temporary materialized views fully implemented
> > and that in my view patch is complete. If there is anything else to
> > add, please let me know, I do not yet have much experience
> > contributing here. What are next steps? Do I just wait for it to be
> > included into Commitfest? Do I add it there myself?
>
> Yes, please add it yourself to the commitfest.
>
> --
> Álvaro Herrera                https://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
http://mitar.tnode.com/
https://twitter.com/mitar_m

tempmatviews-v3.patch (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Mitar
Hi!

One more version of the patch with more deterministic tests.


Mitar

On Thu, Dec 27, 2018 at 10:35 AM Mitar <[hidden email]> wrote:

>
> Hi!
>
> Thanks, I did it.
>
> I am attaching a new version of the patch with few more lines added to tests.
>
> I noticed that there is no good summary of the latest patch, so let me
> make it here:
>
> So the latest version of the patch adds an option for "temporary"
> materialized views. Such materialized views are automatically deleted
> at the end of the session. Moreover, it also modifies the materialized
> view creation logic so that now if any of the source relations are
> temporary, the final materialized view is temporary as well. This now
> makes materialized views more aligned with regular views.
>
> Tests test that this really works, that refreshing of such views work,
> and that refreshing can also work from a trigger.
>
>
> Mitar
>
> On Thu, Dec 27, 2018 at 5:15 AM Alvaro Herrera <[hidden email]> wrote:
> >
> > On 2018-Dec-27, Mitar wrote:
> >
> > > Hi!
> > >
> > > I made a new version of the patch. I added tests and changes to the
> > > docs and made sure various other aspects of this change for as well. I
> > > think this now makes temporary materialized views fully implemented
> > > and that in my view patch is complete. If there is anything else to
> > > add, please let me know, I do not yet have much experience
> > > contributing here. What are next steps? Do I just wait for it to be
> > > included into Commitfest? Do I add it there myself?
> >
> > Yes, please add it yourself to the commitfest.
> >
> > --
> > Álvaro Herrera                https://www.2ndQuadrant.com/
> > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
>
> --
> http://mitar.tnode.com/
> https://twitter.com/mitar_m


--
http://mitar.tnode.com/
https://twitter.com/mitar_m

tempmatviews-v4.patch (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Andreas Karlsson
On 12/28/18 8:48 AM, Mitar wrote:> One more version of the patch with
more deterministic tests.

Her is quick initial review. I will do more testing later.

It applies builds and passes the tests.

The feature seems useful and also improves consistency, if we have
temporary tables and temporary views there should logically also be
temporary materialized tables.

As for you leaving out ON COMMIT I feel that it is ok since of the
existing options only really DROP makes any sense (you cannot truncate
materialized views) and since temporary views do not have any ON COMMIT
support.

= Comments on the code

The changes to the code are small and look mostly correct.

In create_ctas_internal() why do you copy the relation even when you do
not modify it?

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.

Andreas

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Mitar
Hi!

On Fri, Jan 11, 2019 at 8:51 AM Andreas Karlsson <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Andreas Karlsson
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

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Tom Lane-2
Andreas Karlsson <[hidden email]> writes:
> On 1/11/19 8:47 PM, Mitar wrote:
>>> Is it really ok to just remove SECURITY_RESTRICTED_OPERATION from
>>> ExecCreateTableAs()?

>> 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."

> 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.

Indeed.  I don't buy the argument that this should work differently
for temp views.  The fact that they're only accessible in the current
session is no excuse for that: security considerations still matter,
because you can have different privilege contexts within a single
session (consider SECURITY DEFINER functions etc).

What is the stumbling block to just leaving that alone?

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Andreas Karlsson
On 1/17/19 4:57 PM, Tom Lane wrote:

> Andreas Karlsson <[hidden email]> writes:
>> On 1/11/19 8:47 PM, Mitar wrote:
>>>> Is it really ok to just remove SECURITY_RESTRICTED_OPERATION from
>>>> ExecCreateTableAs()?
>
>>> 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."
>
>> 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.
>
> Indeed.  I don't buy the argument that this should work differently
> for temp views.  The fact that they're only accessible in the current
> session is no excuse for that: security considerations still matter,
> because you can have different privilege contexts within a single
> session (consider SECURITY DEFINER functions etc).
>
> What is the stumbling block to just leaving that alone?

I think the issue Mitar ran into is that the temporary materialized view
is created in the rStartup callback of the receiver which happens after
SECURITY_RESTRICTED_OPERATION is set in ExecCreateTableAs(), so the
creation of the view itself is denied.

 From a cursory glance it looks like it would be possible to move the
setting of SECURITY_RESTRICTED_OPERATION to inside the rStartup
callabck, other than that the code for resetting the security context
might get a bit ugly. Do you see any flaws with that solution?

Andreas

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Tom Lane-2
Andreas Karlsson <[hidden email]> writes:
> On 1/17/19 4:57 PM, Tom Lane wrote:
>> What is the stumbling block to just leaving that alone?

> I think the issue Mitar ran into is that the temporary materialized view
> is created in the rStartup callback of the receiver which happens after
> SECURITY_RESTRICTED_OPERATION is set in ExecCreateTableAs(), so the
> creation of the view itself is denied.

Hm.

>  From a cursory glance it looks like it would be possible to move the
> setting of SECURITY_RESTRICTED_OPERATION to inside the rStartup
> callabck, other than that the code for resetting the security context
> might get a bit ugly. Do you see any flaws with that solution?

Don't think that works: the point here is to restrict what can happen
during planning/execution of the view query, so letting planning and
query startup happen first is no good.

Creating the view object inside the rStartup callback is itself pretty
much of a kluge; you'd expect that to happen earlier.  I think the
reason it was done that way was it was easier to find out the view's
column set there, but I'm sure we can find another way --- doing the
object creation more like a regular view does it is the obvious approach.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Andreas Karlsson
In reply to this post by Mitar
On 1/11/19 8:47 PM, Mitar wrote:
> Thanks for doing the review!

I did some functional testing today and everything seems to work as
expected other than that the tab completion for psql seems to be missing.

Andreas


Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Mitar
In reply to this post by Andreas Karlsson
Hi!

On Thu, Jan 17, 2019 at 9:53 AM Andreas Karlsson <[hidden email]> wrote:
> > What is the stumbling block to just leaving that alone?
>
> I think the issue Mitar ran into is that the temporary materialized view
> is created in the rStartup callback of the receiver which happens after
> SECURITY_RESTRICTED_OPERATION is set in ExecCreateTableAs(), so the
> creation of the view itself is denied.

Yes, the error without that change is:

ERROR:  cannot create temporary table within security-restricted operation


Mitar

--
http://mitar.tnode.com/
https://twitter.com/mitar_m

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Mitar
In reply to this post by Andreas Karlsson
Hi!

On Thu, Jan 17, 2019 at 2:40 PM Andreas Karlsson <[hidden email]> wrote:
> I did some functional testing today and everything seems to work as
> expected other than that the tab completion for psql seems to be missing.

Thanks. I can add those as soon as I figure how. :-)

So what are next steps here besides tab autocompletion? It is OK to
remove that security check? If I understand correctly, there are some
general refactoring of code Tom is proposing, but I am not sure if I
am able to do that/understand that.


Mitar

--
http://mitar.tnode.com/
https://twitter.com/mitar_m

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Andreas Karlsson
On 1/18/19 2:53 AM, Mitar wrote:> On Thu, Jan 17, 2019 at 2:40 PM
Andreas Karlsson <[hidden email]> wrote:
>> I did some functional testing today and everything seems to work as
>> expected other than that the tab completion for psql seems to be missing.
>
> Thanks. I can add those as soon as I figure how. :-)

These rules are usually pretty easy to add. Just take a look in
src/bin/psql/tab-complete.c to see how it is usually done.

> So what are next steps here besides tab autocompletion? It is OK to
> remove that security check? If I understand correctly, there are some
> general refactoring of code Tom is proposing, but I am not sure if I
> am able to do that/understand that.

No, I do not think it is ok to remove the check without a compelling
argument for why the usability we gain from this check is not worth it.
Additionally I agree with Tom that the way the code is written currently
is confusing so this refactoring would most likely be a win even without
your patch.

I might take a stab at refactoring this myself this weekend. Hopefully
it is not too involved.

Andreas

Reply | Threaded
Open this post in threaded view
|

Re: Feature: temporary materialized views

Mitar
Hi!

On Fri, Jan 18, 2019 at 7:18 AM Andreas Karlsson <[hidden email]> wrote:
> These rules are usually pretty easy to add. Just take a look in
> src/bin/psql/tab-complete.c to see how it is usually done.

Thanks. I have added the auto-complete and attached a new patch.

> I might take a stab at refactoring this myself this weekend. Hopefully
> it is not too involved.

That would be great! I can afterwards update the patch accordingly.


Mitar

--
http://mitar.tnode.com/
https://twitter.com/mitar_m

tempmatviews-v5.patch (18K) Download Attachment
12