Hi!
Thank you very much for review.
1.
>you can put it into one INSERT. like
>INSERT INTO sales_range VALUES (1, 'May', 1000, '2022-01-31'),
>(1, 'May', 1000, '2022-01-31');
>which can make the regress test faster.
>(apply the logic to other places in
src/test/regress/sql/partition_merge.sql)
Test changed.
2.
>+ errmsg("partition of hash-partitioned table cannot be merged")));
>This error case doesn't seem to have a related test, and adding one
>would be great.
Added test for hash partitioned table.
3.
>per
>https://www.postgresql.org/docs/current/error-message-reporting.html
>"The extra parentheses were required before PostgreSQL version 12, but
>are now optional."
>so now you can remove the extra parentheses.
Extra parentheses removed.
4.
>we can make the first error message like the second one.
>errmsg("\"%s\" is not a partition of \"%s\"....)
Error message
errmsg("relation \"%s\" is not a partition of relation \"%s\""
occurs in two more places in the code.
I think it's better to keep this error message (for consistency).
5.
>+ errmsg("list of new partitions should contain at least two items")));
>This also seems to have no tests.
>adding a dummy one should be ok.
Test added.
6.
>We added List *partlist into PartitionCmd
>typedef struct PartitionCmd
>we should use
>cmd->partlist = NIL;
>instead of
>cmd->partlist = NULL;
>We also need comments explaining PartitionCmd.name
>meaning for ALTER TABLE MERGE PARTITIONS INTO?
Fixed.
7.
>transformPartitionCmdForMerge
>+ partOid = RangeVarGetRelid(name, NoLock, false);
>here "NoLock" seems not right?
AccessExclusiveLock on partitioned table protects only the DEFAULT
partition. Fixed.
P.S. Similar changes were made for the second commit with SPLIT PARTITION.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.com
