From: | Junwang Zhao <zhjwpku(at)gmail(dot)com> |
---|---|
To: | Amul Sul <sulamul(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Support specify tablespace for each merged/split partition |
Date: | 2024-08-06 10:28:16 |
Message-ID: | CAEG8a3+uLedNhZMR20ZzZX40LaGW=YMja_WX3ym30ahRitCZqQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Aug 6, 2024 at 5:34 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
>
> 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.
> ---
I changed to this with minor changes.
> > >
> > > +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.
Thanks for your confirmation.
Attached v2 addressed all the problems you mentioned, thanks.
>
> >
> > I have added you as a reviewer, hope you don't mind.
>
> Thank you.
>
> Regards,
> Amul
--
Regards
Junwang Zhao
Attachment | Content-Type | Size |
---|---|---|
v2-0001-support-specify-tablespace-for-each-merged-split.patch | application/octet-stream | 13.0 KB |
v2-0002-fix-stale-comments.patch | application/octet-stream | 1.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Ranier Vilela | 2024-08-06 11:23:15 | Re: Memory growth observed with C++ application consuming libpq.dll on Windows |
Previous Message | Alexander Korotkov | 2024-08-06 10:23:35 | Re: [HACKERS] make async slave to wait for lsn to be replayed |