On 2016/09/22 19:10, Ashutosh Bapat wrote: > On Thu, Sep 22, 2016 at 1:02 PM, Ashutosh Bapat > <ashutosh.ba...@enterprisedb.com> wrote: >> For list partitions, the ListInfo stores the index maps for values >> i.e. the index of the partition to which the value belongs. Those >> indexes are same as the indexes in partition OIDs array and come from >> the catalogs. In case a user creates two partitioned tables with >> exactly same lists for partitions but specifies them in a different >> order, the OIDs are stored in the order specified. This means that >> index array for these tables come out different. equal_list_info() >> works around that by creating an array of mappings and checks whether >> that mapping is consistent for all values. This means we will create >> the mapping as many times as equal_list_info() is called, which is >> expected to be more than the number of time >> RelationBuildPartitionDescriptor() is called. Instead, if we >> "canonicalise" the indexes so that they come out exactly same for >> similarly partitioned tables, we build the mapping only once and >> arrange OIDs accordingly. >> >> Here's patch to do that. I have ran make check with this and it didn't >> show any failure. Please consider this to be included in your next set >> of patches.
Thanks. It seems like this will save quite a few cycles for future users of equal_list_info(). I will incorporate it into may patch. With this patch, the mapping is created *only once* during RelationBuildPartitionDesc() to assign canonical indexes to individual list values. The partition OID array will also be rearranged such that using the new (canonical) index instead of the old catalog-scan-order-based index will retrieve the correct partition for that value. By the way, I fixed one thinko in your patch as follows: - result->oids[i] = oids[mapping[i]]; + result->oids[mapping[i]] = oids[i]; That is, copy an OID at a given position in the original array to a *mapped position* in the result array (instead of the other way around). > The patch has an if condition as statement by itself > + if (l1->null_index != l2->null_index); > return false; > > There shouldn't be ';' at the end. It looks like in the tests you have > added the function always bails out before it reaches this statement. There is no user of equal_list_info() such that the above bug would have caused a regression test failure. Maybe a segfault crash (due to dangling pointer into partition descriptor's listinfo after the above function would unintentionally return false to equalPartitionDescs() which is in turn called by RelationClearRelation()). Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers