Hello,
PFA the updated patch.
Dependent patch default_partition_v17.patch [1]
This patch adds regression tests and removes the separate function to
get default partition qual.
On Mon, May 29, 2017 at 3:22 PM, Jeevan Ladhe
<jeevan.ladhe@enterprisedb.com> wrote:
> Hi Beena,
>
> I went through your patch, and here are some of my comments:
>
> - For generating a qual for default range partition, instead of scanning for
> all
> the children and collecting all the boundspecs, how about creating and
> negating
> an expression from the lists of lowerdatums and upperdatums in boundinfo.
>
Unlike list, range partition can be for multiple columns and the
expressions get complicated. I have used the same logic of looping
through different partitions and negating the ORed expr. However, this
is now done under get_qual_for_range.
> - Wrong comment:
> + int default_index; /* Index of the default list partition. */
Comment is made generic in the dependent patch.
>
> - Suggested by Robert earlier on default list partitioning thread, instead
> of
> abbreviating is_def/found_def use is(found)_default etc.
corrected.
>
> - unrelated change:
> - List *all_parts;
> + List *all_parts;
>
undone.
> - typo: partiton should be partition, similar typo at other places too.
> + * A non-default range partiton table does not currently allow partition
> keys
>
rectified.
> - Useless hunk for this patch:
> - Oid defid;
> + Oid defid;
undone.
>
> - better to use IsA here, instead of doing explicit check:
> + bound->content[i] = (datum->type == T_DefElem)
> + ? RANGE_DATUM_DEFAULT
Modified
>
> - It is better to use head of either lowerdatums or upperdatums list to
> verify
> if its a default partition and get rid of the constant PARTITION_DEFAULT
> altogether.
>
modified this part as necessary.
> - In the function get_qual_from_partbound() is_def is always going to be
> false
> for range partition, the function get_qual_for_range can be directly passed
> false instead.
>
> - Following comment for function get_qual_for_range_default() implies that
> this
> function returns bool, but the actually it returns a List.
> + *
> + * If DEFAULT is the only partiton for the table then this returns TRUE.
> + *
>
Updated.
[1] http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg315573.html
--
Beena Emerson
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
