Re: Default Partition for Range

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(dot)ladhe(at)enterprisedb(dot)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(at)postgresql(dot)org/msg315573.html

--

Beena Emerson

EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company