Horton

Thread

Commits

  1. Code review for make_partition_op_expr.

  2. Fix multi-column range partitioning constraints.

  1. fix hard-coded index in make_partition_op_expr

    Jeevan Ladhe <jeevan.ladhe@enterprisedb.com> — 2017-05-17T09:38:26Z

    Hi,

While browsing through the partitioning code, I noticed that a recent commit
f8bffe9e6d700fd34759a92e47930ce9ba7dcbd5, which fixes multi-column range
partitioning constraints, introduced a function make_partition_op_expr, that
takes keynum as a input parameter to identify the index of the partition
key.
In case of range partition we can have multiple partition keys but for list
partition we have only one. Considering that, I think following code does
not
cause any side-effect logically(and may be a oversight while moving the code
from function get_qual_for_list to this function):

saopexpr->inputcollid = key->partcollation[0];
saopexpr->args = list_make2(arg1, arg2);

But as we have keynum now, should we be using it to index
key->partcollation,
instead of a hard coded '0'.

I tried to look around in list partitioning and hard coded '0' is used at
all
the places, but that code is either list specific or does not have
availability
of partitioned key index.

Attached patch does this small change in make_partition_op_expr.
Another way is to, have an Assert in case of PARTITION_STRATEGY_LIST:
Assert(keynum != 0);

PFA.

Regards,
Jeevan Ladhe

  2. Re: fix hard-coded index in make_partition_op_expr

    Robert Haas <robertmhaas@gmail.com> — 2017-05-17T18:43:12Z

    On Wed, May 17, 2017 at 5:38 AM, Jeevan Ladhe
<jeevan.ladhe@enterprisedb.com> wrote:
> While browsing through the partitioning code, I noticed that a recent commit
> f8bffe9e6d700fd34759a92e47930ce9ba7dcbd5, which fixes multi-column range
> partitioning constraints, introduced a function make_partition_op_expr, that
> takes keynum as a input parameter to identify the index of the partition
> key.
> In case of range partition we can have multiple partition keys but for list
> partition we have only one. Considering that, I think following code does
> not
> cause any side-effect logically(and may be a oversight while moving the code
> from function get_qual_for_list to this function):
>
> saopexpr->inputcollid = key->partcollation[0];
> saopexpr->args = list_make2(arg1, arg2);
>
> But as we have keynum now, should we be using it to index
> key->partcollation,
> instead of a hard coded '0'.

Agreed.  Committed your patch.

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

  3. Re: fix hard-coded index in make_partition_op_expr

    Jeevan Ladhe <jeevan.ladhe@enterprisedb.com> — 2017-05-17T18:56:58Z

    On Thu, May 18, 2017 at 12:13 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> Agreed.  Committed your patch.
>

 Thanks Robert!

  4. Re: fix hard-coded index in make_partition_op_expr

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2017-05-18T02:04:00Z

    On 2017/05/18 3:43, Robert Haas wrote:
> On Wed, May 17, 2017 at 5:38 AM, Jeevan Ladhe
> <jeevan.ladhe@enterprisedb.com> wrote:
>> While browsing through the partitioning code, I noticed that a recent commit
>> f8bffe9e6d700fd34759a92e47930ce9ba7dcbd5, which fixes multi-column range
>> partitioning constraints, introduced a function make_partition_op_expr, that
>> takes keynum as a input parameter to identify the index of the partition
>> key.
>> In case of range partition we can have multiple partition keys but for list
>> partition we have only one. Considering that, I think following code does
>> not
>> cause any side-effect logically(and may be a oversight while moving the code
>> from function get_qual_for_list to this function):
>>
>> saopexpr->inputcollid = key->partcollation[0];
>> saopexpr->args = list_make2(arg1, arg2);
>>
>> But as we have keynum now, should we be using it to index
>> key->partcollation,
>> instead of a hard coded '0'.
> 
> Agreed.  Committed your patch.

Me too, thanks both.

Regards,
Amit