On Wed, May 10, 2017 at 11:39 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Wed, May 3, 2017 at 9:09 AM, amul sul <sula...@gmail.com> wrote: >> Fixed in the attached version. > > +[ PARTITION BY { HASH | RANGE | LIST } ( { <replaceable > class="parameter">column_name</replaceable> | ( <replaceable > class="parameter">expression</replaceable> ) } [ COLLATE <replaceable > > In the department of severe nitpicking, I would have expected this to > either use alphabetical order (HASH | LIST | RANGE) or to add the new > method at the end on the theory that we probably did the important > ones first (RANGE | LIST | HASH).
Importance is subjective, so may be we should arrange them in alphabetical order, to keep the list in some order and be consistent everywhere in the code and documentation. > More broadly, I wonder why we're > cramming this into the datums arrays instead of just adding another > field to PartitionBoundInfoData that is only used by hash > partitioning. It would be good if we store datums corresponding to partition bounds in the same place. So that we don't have to handle hash partition specially in all the places where we handle partition bound datums. We already do that for list and range partitions. May be we want to continue doing so for hash as well. In my comments to Amul's latest patch, I have described a possibility that partition_bounds_equal() need not compare all entries in the datums array. It can just compare greated modulus and the indexes array from given two partition bounds to check whether they are equal. If that works well, we will probably address your complaint about DatumIsEqual() in a different manner. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers