Re: Support specify tablespace for each merged/split partition

On Mon, Aug 5, 2024 at 9:05 PM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
>
> Hi Amul,
>
> Thanks for your review.
>
> On Mon, Aug 5, 2024 at 8:38 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
> >
> > On Mon, Jul 15, 2024 at 11:19 AM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
> > >
> >[...]
> > static Relation
> > -createPartitionTable(RangeVar *newPartName, Relation modelRel,
> > - AlterTableUtilityContext *context)
> > +createPartitionTable(RangeVar *newPartName, char *tablespacename,
> > +
> >
> > The comment should mention the tablespace setting in the same way it
> > mentions the access method.
>
> I'm not good at wording, can you give some advice?

My suggestion is to rewrite the third paragraph as follows, but
someone else might have a better version:
---
The new partitions will also be created in the same tablespace as the parent
if not specified. Also, this function sets the new partition access method
same as parent table access methods (similarly to CREATE TABLE ... PARTITION
OF). It checks that parent and child tables have compatible persistence.
---
> >
> > +SELECT tablename, tablespace FROM pg_tables
> > + WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
> > + ORDER BY tablename, tablespace;
> > + tablename | tablespace
> > +-----------+------------------
> > + t |
> > + tp_0_2 | regress_tblspace
> > +(2 rows)
> > +
> > +SELECT tablename, indexname, tablespace FROM pg_indexes
> > + WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
> > + ORDER BY tablename, indexname, tablespace;
> > + tablename | indexname | tablespace
> > +-----------+-------------+------------
> > + t | t_pkey |
> > + tp_0_2 | tp_0_2_pkey |
> > +(2 rows)
> > +
> >
> > This seems problematic to me. The index should be in the same
> > tablespace as the table.
>
> I'm not sure about this, it seems to me that partition index will alway
> inherit the tablespaceId of its parent(see generateClonedIndexStmt),
> do you think we should change the signature of this function?
>
> One thing worth mentioning is that for UNIQUE/PRIMARY KEY,
> it allows setting *USING INDEX TABLESPACE tablespace_name*,
> I don't think we should change the index tablespace in this case,
> what do you think?
>

I think you are correct; my understanding is a bit hazy.

>
> I have added you as a reviewer, hope you don't mind.

Thank you.

Regards,
Amul