On Thu, Jan 19, 2017 at 12:15 AM, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: > 0001-Fix-a-bug-of-insertion-into-an-internal-partition.patch > > Since implicit partition constraints are not inherited, an internal > partition's constraint was not being enforced when targeted directly. > So, include such constraint when setting up leaf partition result > relations for tuple-routing.
Committed. > 0002-Set-ecxt_scantuple-correctly-for-tuple-routing.patch > > In 2ac3ef7a01df859c62d0a02333b646d65eaec5ff, we changed things so that > it's possible for a different TupleTableSlot to be used for partitioned > tables at successively lower levels. If we do end up changing the slot > from the original, we must update ecxt_scantuple to point to the new one > for partition key of the tuple to be computed correctly. > > Last posted here: > https://www.postgresql.org/message-id/99fbfad6-b89b-9427-a6ca-197aad98c48e%40lab.ntt.co.jp Why does FormPartitionKeyDatum need this? Could that be documented in a comment in here someplace, perhaps the header comment to FormPartitionKeyDatum? > 0003-Fix-RETURNING-to-work-correctly-after-tuple-routing.patch > > In ExecInsert(), do not switch back to the root partitioned table > ResultRelInfo until after we finish ExecProcessReturning(), so that > RETURNING projection is done using the partition's descriptor. For > the projection to work correctly, we must initialize the same for > each leaf partition during ModifyTableState initialization. Committed. > 0004-Fix-some-issues-with-views-and-partitioned-tables.patch > > Automatically updatable views failed to handle partitioned tables. > Once that's fixed, WITH CHECK OPTIONS wouldn't work correctly without > the WCO expressions having been suitably converted for each partition > (think applying map_partition_varattnos to Vars in the WCO expressions > just like with partition constraint expressions). The changes to execMain.c contain a hunk which has essentially the same code twice. That looks like a mistake. Also, the patch doesn't compile because convert_tuples_by_name() takes 3 arguments, not 4. > 0005-Fix-some-wrong-thinking-in-check_new_partition_bound.patch > > Because a given range bound in the PartitionBoundInfo.datums array > is sometimes a range lower bound and upper bound at other times, we > must be careful when assuming which, especially when interpreting > the result of partition_bound_bsearch which returns the index of the > greatest bound that is less than or equal to probe. Due to an error > in thinking about the same, the relevant code in > check_new_partition_bound() caused invalid partition (index = -1) > to be chosen as the partition being overlapped. > > Last posted here: > https://www.postgresql.org/message-id/603acb8b-5dec-31e8-29b0-609a68aac50f%40lab.ntt.co.jp } + /* + * If equal has been set to true or if there is no "gap" + * between the bound at off1 and that at off1 + 1, the new + * partition will overlap some partition. In the former + * case, the new lower bound is found to be equal to the + * bound at off1, which could only ever be true if the + * latter is the lower bound of some partition. It's + * clear in such a case that the new partition overlaps + * that partition, whose index we get using its upper + * bound (that is, using the bound at off1 + 1). + */ else Stylistically, we usually avoid this, or at least I do. The comment should go inside the "else" block. But it looks OK apart from that, so committed with a little rephrasing and reformatting of the comment. > 0006-Avoid-tuple-coversion-in-common-partitioning-cases.patch > > Currently, the tuple conversion is performed after a tuple is routed, > even if the attributes of a target leaf partition map one-to-one with > those of the root table, which is wasteful. Avoid that by making > convert_tuples_by_name() return a NULL map for such cases. + Assert(!consider_typeid && indesc->tdhasoid == outdesc->tdhasoid); I think you mean Assert(consider_typeid || indesc->tdhasoid == outdesc->tdhasoid); But I wonder why we don't instead just change this function to consider tdhasoid rather than tdtypeid. I mean, if the only point of comparing the type OIDs is to find out whether the table-has-OIDs setting matches, we could instead test that directly and avoid needing to pass an extra argument. I wonder if there's some other reason this code is there which is not documented in the comment... > 0007-Avoid-code-duplication-in-map_partition_varattnos.patch > > Code to map attribute numbers in map_partition_varattnos() duplicates > what convert_tuples_by_name_map() does. Avoid that as pointed out by > Álvaro Herrera. > > Last posted here: > https://www.postgresql.org/message-id/9ce97382-54c8-deb3-9ee9-a2ec271d866b%40lab.ntt.co.jp Committed. > 0008-Avoid-DROP-TABLE-.-CASCADE-in-more-partitioning-test.patch > > This is the new one. There were quite a few commits recently to fix the > breakage in regression tests due to not using ORDER BY in queries on > system catalogs and using DROP TABLE ... CASCADE. There were still some > instances of the latter in create_table.sql and alter_table.sql. Committed. Phew. Thanks for all the patches, sorry I'm having trouble keeping up. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
