On 22 June 2017 at 01:41, Robert Haas <robertmh...@gmail.com> wrote: >>> + for (i = 0; i < num_rels; i++) >>> + { >>> + ResultRelInfo *resultRelInfo = &result_rels[i]; >>> + Relation rel = resultRelInfo->ri_RelationDesc; >>> + Bitmapset *expr_attrs = NULL; >>> + >>> + pull_varattnos((Node *) rel->rd_partcheck, 1, &expr_attrs); >>> + >>> + /* Both bitmaps are offset by FirstLowInvalidHeapAttributeNumber. >>> */ >>> + if (bms_overlap(expr_attrs, GetUpdatedColumns(resultRelInfo, >>> estate))) >>> + return true; >>> + } >>> >>> This seems like an awfully expensive way of performing this test. >>> Under what circumstances could this be true for some result relations >>> and false for others; >> >> One resultRelinfo can have no partition key column used in its quals, >> but the next resultRelinfo can have quite different quals, and these >> quals can have partition key referred. This is possible if the two of >> them have different parents that have different partition-key columns. > > Hmm, true. So if we have a table foo that is partitioned by list (a), > and one of its children is a table bar that is partitioned by list > (b), then we need to consider doing tuple-routing if either column a > is modified, or if column b is modified for a partition which is a > descendant of bar. But visiting that only requires looking at the > partitioned table and those children that are also partitioned, not > all of the leaf partitions as the patch does.
The main concern is that the non-leaf partitions are not open (except root), so we would need to open them in order to get the partition key of the parents of update resultrels (or get only the partition key atts and exprs from pg_partitioned_table). There can be multiple approaches to finding partition key columns. Approach 1 : When there are a few update result rels and a large partition tree, we traverse from each of the result rels to their ancestors , and open their ancestors (get_partition_parent()) to get the partition key columns. For result rels having common parents, do this only once. Approach 2 : If there are only a few partitioned tables, and large number of update result rels, it would be easier to just open all the partitioned tables and form the partition key column bitmap out of all their partition keys. If the bitmap does not have updated columns, that's not a partition-key-update. So for typical non-partition-key updates, just opening the partitioned tables will suffice, and so that would not affect performance of normal updates. But if the bitmap has updated columns, we can't conclude that it's a partition-key-update, otherwise it would be false positive. We again need to further check whether the update result rels belong to ancestors that have updated partition keys. Approach 3 : In RelationData, in a new bitmap field (rd_partcheckattrs ?), store partition key attrs that are used in rd_partcheck . Populate this field during generate_partition_qual(). So to conclude, I think, we can do this : Scenario 1 : Only one partitioned table : the root; rest all are leaf partitions. In this case, it is definitely efficient to just check the root partition key, which will be sufficient. Scenario 2 : There are few non-leaf partitioned tables (3-4) : Open those tables, and follow 2nd approach above: If we don't find any updated partition-keys in any of them, well and good. If we do find, failover to approach 3 : For each of the update resultrels, use the new rd_partcheckattrs bitmap to know if it uses any of the updated columns. This would be faster than pulling up attrs from the quals like how it was done in the patch. -- Thanks, -Amit Khandekar EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers