Hi Jaime, On 2016/11/08 2:15, Jaime Casanova wrote: > On 28 October 2016 at 02:53, Amit Langote <langote_amit...@lab.ntt.co.jp> > wrote: > > I started to review the functionality of this patch, so i applied all > 9 patches. After that i found this warning, which i guess is because > it needs a cast.
Thanks a ton for reviewing! > After that, i tried a case that i think is important: to partition an > already existing table. Because there is no ALTER TABL SET PARTITION > or something similar (which i think makes sense because such a command > would need to create the partitions and move the rows to follow the > rule that there is no rows in a parent table). > > So, what i tried was: > > 1) rename original table > 2) create a new partitioned table with old name > 3) attach old table as a partition with bounds outside normal bounds > and no validate > > the idea is to start attaching valid partitions and delete and insert > rows from the invalid one (is there a better way of doing that?), that > will allow to partition a table easily. So, step 3 creates a partition that is basically unbounded. From there, it seems you want to create partitions with proper bounds, moving data into them as they are created. It seems like a job of some redistribution command (split partition?) which is currently unsupported. > So far so good, until i decided points 1 to 3 should happen inside a > transaction to make things transparent to the user. > > Attached is script that shows the failure when trying it: > > script 1 (failing_test_1.sql) fails the assert > "Assert(RelationGetPartitionKey(parentRel) != NULL);" in > transformAttachPartition() at src/backend/parser/parse_utilcmd.c:3164 Thanks for the test. This test uncovered a bug which I have fixed in my local repository. Given the fix the above will work, although I see that it's not the best way to do what you want to do. > After that i tried the same but with an already partitioned (via > inheritance) table and got this (i did this first without a > transaction, doing it with a transaction will show the same failure as > before): > > script 2 (failing_test_2.sql) fails the assert > "Assert(childrte->relkind == RELKIND_PARTITIONED_TABLE);" in > expand_inherited_rte_internal() at > src/backend/optimizer/prep/prepunion.c:1551 This again was an oversight/bug in the patch. It's not supported to combine old-style inheritance partitioning with the new partitioned tables. In fact, ATTACH PARTITION prevents adding a regular inheritance parent as partition. After fixing the bug, you would instead get this error: alter table prueba attach partition prueba_old for values start (unbounded, unbounded) end (2008,1) no validate; ERROR: cannot attach regular inheritance parent as partition In this case, you should instead create prueba_old partition hierarchy using the new partitioning commands and then attach the same. > PS: i don't like the START END syntax but i don't have any ideas > there... except, there was a reason not to use expressions (like a > CHECK constraint?) Expression syntax is too unrestricted. What's to prevent users from using completely different check constraint expressions from partition to partition (not that any users deliberately try do that)? With the new partitioning syntax, you specify the partitioning columns once when creating the partitioned parent and then specify, using partitioning method specific syntax, the bounds for every partition of the parent. That allows to capture the partition metadata in a form that is more suitable to implement other features related to partitioning. That said, I think it's always a challenge to come up with a syntax that is universally acceptable. For example, the recent discussion about whether to allow inclusive/exclusive to be specified for START and END bounds of range partitions or always assume inclusive start and exclusive end [1]. Thanks, Amit [1] https://www.postgresql.org/message-id/CA%2BTgmoaKOycHcVoed%2BF3fk-z6xUOeysQFG6HT%3Doucw76bSMHCQ%40mail.gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
