don't create storage when unnecessary

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

don't create storage when unnecessary

Alvaro Herrera-9
Some time ago, after partitioned indexes had been pushed, I realized
that even though I didn't want them to have relfilenodes, they did.  And
looking closer I noticed that *a lot* of relation kinds that didn't need
relfilenodes, had them anyway.

This patch fixes that; if no relfilenode is needed, it's not created.

I didn't verify pg_upgrade behavior across this commit.  Maybe something
needs tweaking there.


PS: I think it'd be worth following up with this ...
https://postgr.es/m/CAFjFpRcfzs+yst6YBCseD_orEcDNuAr9GUTraZ5GC=AvCYh55Q@...

--
Álvaro Herrera

0001-don-t-create-storage-when-not-necessary.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: don't create storage when unnecessary

Andres Freund
Hi,

On 2018-12-06 18:55:52 -0300, Alvaro Herrera wrote:
> Some time ago, after partitioned indexes had been pushed, I realized
> that even though I didn't want them to have relfilenodes, they did.  And
> looking closer I noticed that *a lot* of relation kinds that didn't need
> relfilenodes, had them anyway.
>
> This patch fixes that; if no relfilenode is needed, it's not created.
>
> I didn't verify pg_upgrade behavior across this commit.  Maybe something
> needs tweaking there.

Hm, that generally sounds like a good plan. Could we back this up with
tests in misc_sanity.sql or such?

- Andres

Reply | Threaded
Open this post in threaded view
|

Re: don't create storage when unnecessary

Michael Paquier-2
In reply to this post by Alvaro Herrera-9
On Thu, Dec 06, 2018 at 06:55:52PM -0300, Alvaro Herrera wrote:

> Some time ago, after partitioned indexes had been pushed, I realized
> that even though I didn't want them to have relfilenodes, they did.  And
> looking closer I noticed that *a lot* of relation kinds that didn't need
> relfilenodes, had them anyway.
>
> This patch fixes that; if no relfilenode is needed, it's not created.
>
> I didn't verify pg_upgrade behavior across this commit.  Maybe something
> needs tweaking there.
>
> PS: I think it'd be worth following up with this ...
> https://postgr.es/m/CAFjFpRcfzs+yst6YBCseD_orEcDNuAr9GUTraZ5GC=AvCYh55Q@...
A macro makes sense to control that.  Now I have to admit that I don't
like your solution.  Wouldn't it be cleaner to assign InvalidOid to
relfilenode in such cases?  The callers of heap_create would need to be
made smarter when they now pass down a relfilenode (looking at you,
DefineIndex!), but that seems way more consistent to me.  Some tests
would also be welcome.
--
Michael

signature.asc (849 bytes) Download Attachment