Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
От | Dmitry Koval |
---|---|
Тема | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |
Дата | |
Msg-id | 67ecf5fb-e63d-4284-be75-d0479eef81aa@postgrespro.ru обсуждение исходный текст |
Ответ на | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands (jian he <jian.universality@gmail.com>) |
Ответы |
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |
Список | pgsql-hackers |
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
Вложения
В списке pgsql-hackers по дате отправления: