Re: Support specify tablespace for each merged/split partition

On 2024/08/06 19:28, Junwang Zhao wrote:
> Attached v2 addressed all the problems you mentioned, thanks.

Thanks for updating the patches!

In the ALTER TABLE documentation, v1 patch updated the syntax, but the descriptions for MERGE and SPLIT should also be updated to explain the tablespace of new partitions.

+ char *tablespacename; /* name of tablespace, or NULL for default */
PartitionBoundSpec *bound; /* FOR VALUES, if attaching */

This is not the fault of v1 patch, but the comment "if attaching" seems incorrect.

I also noticed the comment for SinglePartitionSpec refers to a different struct name, PartitionDesc. This should be corrected, and it might be better to include this fix in the v2 patch.

- * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH PARTITION commands
+ * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH/MERGE/SPLIT PARTITION commands

How about changing it to "info for ALTER TABLE ATTACH/DETACH/MERGE/SPLIT and ALTER INDEX ATTACH commands" for more precision?

- RangeVar *name; /* name of partition to attach/detach */
+ RangeVar *name; /* name of partition to
+ * attach/detach/merge/split */

In the case of MERGE, it refers to the name of the partition that the command merges into. So, would "merge into" be more appropriate than just "merge" in this comment?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION