On Wed, Jun 7, 2017 at 2:08 AM, Jeevan Ladhe <jeevan.la...@enterprisedb.com> wrote:
> >> >> The code in check_default_allows_bound() to check whether the default >> partition >> has any rows that would fit new partition looks quite similar to the code >> in >> ATExecAttachPartition() checking whether all rows in the table being >> attached >> as a partition fit the partition bounds. One thing that >> check_default_allows_bound() misses is, if there's already a constraint on >> the >> default partition refutes the partition constraint on the new partition, >> we can >> skip the scan of the default partition since it can not have rows that >> would >> fit the new partition. ATExecAttachPartition() has code to deal with a >> similar >> case i.e. the table being attached has a constraint which implies the >> partition >> constraint. There may be more cases which check_default_allows_bound() >> does not >> handle but ATExecAttachPartition() handles. So, I am wondering whether >> it's >> better to somehow take out the common code into a function and use it. We >> will >> have to deal with a difference through. The first one would throw an error >> when >> finding a row that satisfies partition constraints whereas the second one >> would >> throw an error when it doesn't find such a row. But this difference can be >> handled through a flag or by negating the constraint. This would also take >> care >> of Amit Langote's complaint about foreign partitions. There's also another >> difference that the ATExecAttachPartition() queues the table for scan and >> the >> actual scan takes place in ATRewriteTable(), but there is not such queue >> while >> creating a table as a partition. But we should check if we can reuse the >> code to >> scan the heap for checking a constraint. >> >> In case of ATTACH PARTITION, probably we should schedule scan of default >> partition in the alter table's work queue like what >> ATExecAttachPartition() is >> doing for the table being attached. That would fit in the way alter table >> works. > I tried refactoring existing code so that it can be used for default partitioning as well. The code to validate the partition constraints against the table being attached in ATExecAttachPartition() is extracted out into a set of functions. For default partition we reuse those functions to check whether it contains any row that would fit the partition being attached. While creating a new partition, the function to skip validation is reused but the scan portion is duplicated from ATRewriteTable since we are not in ALTER TABLE context. The names of the functions, their declaration will require some thought. There's one test failing because for ATTACH partition the error comes from ATRewriteTable instead of check_default_allows_bounds(). May be we want to use same message in both places or some make ATRewriteTable give a different message while validating default partition. Please review the patch and let me know if the changes look good.
default_partition_refactor_v20.tar.gz
Description: GNU Zip compressed data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
