Hi Dmitry. On Mon, Sep 1, 2025 at 2:04 PM Dmitry Koval <[email protected]> wrote: > Hi! > Thank you for the notes and patch!
Some additional notes from me. 1) src/backend/parser/parse_utilcmd.c includes are not alphabetically ordered here +#include "partitioning/partdesc.h" +#include "partitioning/partbounds.h" 2) There is unicode dash in the comment of ATExecMergePartitions() here. I suggest we should stick to ascii. + /* + * Check ownership of merged partitions — partitions with different + * owners cannot be merged. Also, collect the OIDs of these partitions + * during the check. + */ 3) Regarding 17bcf4f545, I see btnamecmp() is collation-aware. Should we also specify COLLATE "C" every time we do "ORDER BY relname"? 4) This comment sounds misleading. Probably it should say "are contained". +/* + * check_parent_values_in_new_partitions + * + * (function for BY LIST partitioning) + * + * Checks that all values of split partition (with Oid partOid) contains in new + * partitions. + * 5) Given what latter items say, I think the 1. should say "The DEFAULT partition must be at most one." /* * check_partitions_for_split * * Checks new partitions for SPLIT PARTITIONS command: * 1. DEFAULT partition should be one. 6) Regarding the isolation tests. I see we are exercising INSERTs and intra-partition UPDATEs. Should we also try some cross-partition UPDATEs? ------ Regards, Alexander Korotkov Supabase
