Re: Should new partitions inherit their tablespace from their parent?

2018-12-18 Thread Michael Paquier
On Tue, Dec 18, 2018 at 11:40:49AM -0500, Tom Lane wrote: > Agreed, done. Thanks Tom for stepping in! -- Michael signature.asc Description: PGP signature

Re: Should new partitions inherit their tablespace from their parent?

2018-12-18 Thread Tom Lane
Michael Paquier writes: > On Mon, Dec 17, 2018 at 10:13:42PM -0300, Alvaro Herrera wrote: >> I think we need to accept the new output. It is saying that previously >> we would not invoke the hook on SET TABLESPACE for a partitioned table, >> which makes sense, and now we do invoke it, which also

Re: Should new partitions inherit their tablespace from their parent?

2018-12-17 Thread Michael Paquier
On Mon, Dec 17, 2018 at 10:13:42PM -0300, Alvaro Herrera wrote: > I think we need to accept the new output. It is saying that previously > we would not invoke the hook on SET TABLESPACE for a partitioned table, > which makes sense, and now we do invoke it, which also makes sense. Indeed, that

Re: Should new partitions inherit their tablespace from their parent?

2018-12-17 Thread Alvaro Herrera
On 2018-Dec-18, Michael Paquier wrote: > On Tue, Dec 18, 2018 at 09:36:17AM +1300, David Rowley wrote: > > On Tue, 18 Dec 2018 at 08:21, Alvaro Herrera > > wrote: > >> Pushed with those changes. Thanks for the patch! > > > > Thanks for committing it and thanks for reviewing it, Michael. > >

Re: Should new partitions inherit their tablespace from their parent?

2018-12-17 Thread Michael Paquier
On Tue, Dec 18, 2018 at 09:36:17AM +1300, David Rowley wrote: > On Tue, 18 Dec 2018 at 08:21, Alvaro Herrera wrote: >> Pushed with those changes. Thanks for the patch! > > Thanks for committing it and thanks for reviewing it, Michael. Perhaps too early to speak, tests of sepgsql are broken

Re: Should new partitions inherit their tablespace from their parent?

2018-12-17 Thread David Rowley
On Tue, 18 Dec 2018 at 08:21, Alvaro Herrera wrote: > Pushed with those changes. Thanks for the patch! Thanks for committing it and thanks for reviewing it, Michael. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services

Re: Should new partitions inherit their tablespace from their parent?

2018-12-17 Thread Alvaro Herrera
On 2018-Dec-18, David Rowley wrote: > 1. Shouldn't you be using the RangeVarGetRelid() macro instead of > calling RangeVarGetRelidExtended()? This should have been obvious but I didn't notice. > 2. In MergeAttributes(), the parentOids list still exists and is > populated. This is now only used

Re: Should new partitions inherit their tablespace from their parent?

2018-12-17 Thread David Rowley
On Mon, 17 Dec 2018 at 11:07, Alvaro Herrera wrote: > First, I changed the Assert() to use the macro for relations with > storage that I just posted in the other thread that Michael mentioned. > > I then noticed that we're doing a heap_openrv() in the parent relation > and closing it before

Re: Should new partitions inherit their tablespace from their parent?

2018-12-16 Thread David Rowley
On Mon, 17 Dec 2018 at 12:59, Michael Paquier wrote: > Okay, I think that you should add an assertion on > CheckRelationLockedByMe() as MergeAttributes()'s only caller is > DefineRelation(). Better safe than sorry later. Would that not just double up the work that's already done in

Re: Should new partitions inherit their tablespace from their parent?

2018-12-16 Thread Michael Paquier
On Sun, Dec 16, 2018 at 07:07:35PM -0300, Alvaro Herrera wrote: > I'll self-review this again tomorrow, 'cause I now have to run. > - if (!is_partition) > - relation = heap_openrv(parent, > ShareUpdateExclusiveLock); > - else > -

Re: Should new partitions inherit their tablespace from their parent?

2018-12-16 Thread Alvaro Herrera
I didn't like this, so I rewrote it a little bit. First, I changed the Assert() to use the macro for relations with storage that I just posted in the other thread that Michael mentioned. I then noticed that we're doing a heap_openrv() in the parent relation and closing it before MergeAttribute()

Re: Should new partitions inherit their tablespace from their parent?

2018-12-16 Thread David Rowley
On Tue, 11 Dec 2018 at 15:43, Michael Paquier wrote: > + parentrel = heap_openrv(parent, AccessExclusiveLock); > So, in order to determine which tablespace should be used here, an > exclusive lock is taken on the parent because its partition descriptor > gets updated by the addition of the

Re: Should new partitions inherit their tablespace from their parent?

2018-12-10 Thread Michael Paquier
On Mon, Dec 10, 2018 at 10:56:47PM +0900, Michael Paquier wrote: > Cool, thanks for updating the assertion. At quick glance the patch > seems fine to me. Let's keep ATExecSetTableSpaceNoStorage as routine > name as you suggest then. + /* +* For partitions, when no other tablespace

Re: Should new partitions inherit their tablespace from their parent?

2018-12-10 Thread Michael Paquier
On Mon, Dec 10, 2018 at 11:37:16PM +1300, David Rowley wrote: > Likely it would be nice if we had a macro to determine if the relkind > should have storage or not, and then just Assert() we get one of those > into the function. The other thread can deal about that matter I guess. > For now, I've

Re: Should new partitions inherit their tablespace from their parent?

2018-12-10 Thread David Rowley
On Mon, 10 Dec 2018 at 15:22, Michael Paquier wrote: > - Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX); > + Assert(rel->rd_rel->relkind == RELKIND_VIEW || > + rel->rd_rel->relkind == RELKIND_COMPOSITE_TYPE || > + rel->rd_rel->relkind ==

Re: Should new partitions inherit their tablespace from their parent?

2018-12-09 Thread Michael Paquier
On Mon, Dec 10, 2018 at 02:05:29AM +1300, David Rowley wrote: > On Fri, 7 Dec 2018 at 20:15, Michael Paquier wrote: >> -ATExecPartedIdxSetTableSpace(Relation rel, Oid newTableSpace) >> +ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace) >> NoStorage looks strange as routine name for

Re: Should new partitions inherit their tablespace from their parent?

2018-12-09 Thread David Rowley
Thank you for looking at this. On Fri, 7 Dec 2018 at 20:15, Michael Paquier wrote: > -ATExecPartedIdxSetTableSpace(Relation rel, Oid newTableSpace) > +ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace) > NoStorage looks strange as routine name for this case. Would something > like

Re: Should new partitions inherit their tablespace from their parent?

2018-12-06 Thread Michael Paquier
On Sat, Nov 24, 2018 at 10:18:17AM +0900, Michael Paquier wrote: > On Sat, Nov 24, 2018 at 12:32:36PM +1300, David Rowley wrote: >> On Fri, 23 Nov 2018 at 17:03, David Rowley >> wrote: >> > Here's a patch for that. Parking here until January's commitfest. >> >> And another, now rebased atop of

Re: Should new partitions inherit their tablespace from their parent?

2018-11-23 Thread Michael Paquier
On Sat, Nov 24, 2018 at 12:32:36PM +1300, David Rowley wrote: > On Fri, 23 Nov 2018 at 17:03, David Rowley > wrote: > > Here's a patch for that. Parking here until January's commitfest. > > And another, now rebased atop of de38ce1b8 (which I probably should > have waited for). You have just

Re: Should new partitions inherit their tablespace from their parent?

2018-11-23 Thread David Rowley
On Fri, 23 Nov 2018 at 17:03, David Rowley wrote: > Here's a patch for that. Parking here until January's commitfest. And another, now rebased atop of de38ce1b8 (which I probably should have waited for). -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development,

Re: Should new partitions inherit their tablespace from their parent?

2018-11-22 Thread David Rowley
On Fri, 9 Nov 2018 at 06:58, Robert Haas wrote: > On Wed, Nov 7, 2018 at 9:43 PM Michael Paquier wrote: > > On Thu, Nov 08, 2018 at 12:50:40PM +1300, David Rowley wrote: > > > How about we record the tablespace option for the partitioned table in > > > reltablespace instead of saving it as 0.

Re: Should new partitions inherit their tablespace from their parent?

2018-11-08 Thread Robert Haas
On Wed, Nov 7, 2018 at 9:43 PM Michael Paquier wrote: > On Thu, Nov 08, 2018 at 12:50:40PM +1300, David Rowley wrote: > > How about we record the tablespace option for the partitioned table in > > reltablespace instead of saving it as 0. Newly created partitions > > which don't have a TABLESPACE

Re: Should new partitions inherit their tablespace from their parent?

2018-11-07 Thread Michael Paquier
On Thu, Nov 08, 2018 at 12:50:40PM +1300, David Rowley wrote: > How about we record the tablespace option for the partitioned table in > reltablespace instead of saving it as 0. Newly created partitions > which don't have a TABLESPACE mentioned in the CREATE TABLE command > should be created in

Should new partitions inherit their tablespace from their parent?

2018-11-07 Thread David Rowley
Dear Hackers, I've just read the thread in [1] where there was a discussion on a bug fix regarding index partitions not being created in the same tablespace as the partitioned index was created. One paragraph that stood out when reading the thread was: On 8 November 2018 at 09:00, Robert Haas