Thread

  1. Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

    Dmitry Koval <d.koval@postgrespro.ru> — 2025-09-15T22:11:19Z

    Hi Alexander.
    
    Thanks for the notes and corrections!
    
    
    1) src/backend/parser/parse_utilcmd.c includes are not alphabetically 
    ordered here
    +#include "partitioning/partdesc.h"
    +#include "partitioning/partbounds.h"
    
    Fixed.
    
    
    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.
    +    */
    
    Fixed.
    
    
    3) Regarding 17bcf4f545, I see btnamecmp() is collation-aware.  Should 
    we also specify COLLATE "C" every time we do "ORDER BY relname"?
    
    Queries changed.
    
    
    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.
    + *
    
    Changed.
    
    
    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.
    
    Corrected.
    
    
    6) Regarding the isolation tests.  I see we are exercising INSERTs and 
    intra-partition UPDATEs.  Should we also try some cross-partition UPDATEs?
    
    Added simple tests for cross-partition UPDATE.
    
    
    7) Additionally, I've made a numerous and small fixes for grammar to the
    docs directly to the patchset.
    
    Thanks, applied.
    
    -- 
    With best regards,
    Dmitry Koval
    
    Postgres Professional: http://postgrespro.com