Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

Hi!

1.
 > bug:
 > ...
 > now pks_d have values(5) for column i, which would violate the
 > cc check constraint.

Probably we shouldn't use a DEFAULT-partition if it wasn't created by
SPLIT PARTITION command, because DEFAULT-partition can has other
restrictions besides CHECK (e.g. FOREIGN KEY).
Corrected.


2.
 > "ALTER TABLE/INDEX ATTACH/DETACH PARTITION" comments need updated.

Updated.


3.
 > SplitPartitionMoveRows
 >   ereport(ERROR,
 >           errcode(ERRCODE_CHECK_VIOLATION),
 >           errmsg("can not find partition for split partition row"),
 >           errtable(splitRel));
 >will this ever be reachable?

In case correct work, there should be no errors. This is insurance.


4.
 >I am not sure the comment "detached DEFAULT partition" is correct.
 >the "constraint" is not ideal, better word would be "partition
 >constraint" or "partition qual".

Corrected.


5.
 >In this case, we could first create the relations pks_3 and pks_4,
 >then detach the partition pks_34


I think it's better not to change the code, because detachPartitionTable
function call should be before performDeletionCheck.
And performDeletionCheck function should be used as soon as possible.


6.
 >should the newly created partitions be based on the split partition
 >or on the partitioned table?
 >In the current v50 implementation, they are based on the partitioned
 >table, but these newly created partitions based on the split partition
 >also make sense.

Yes, now newly created partitions are based on the partitioned.
I think this is more correct: user after SPLIT can modify some of new
partitions as he wants.


7.
 >this ResetExprContext is not needed, if you are evaluate different
 >ExprState again the same slot. See ExecRelCheck also.

Thanks!


8.
 >I did some regress test refactoring to reduce test size.
 >other miscellaneous refactoring is also attached.

Thanks!


P.S. Excuse me, but I can't answer emails for the next three weeks.

-- 
With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com