Re: Default Partition for Range

Hello Dilip,

On Wed, Jun 21, 2017 at 6:27 PM, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> On Tue, Jun 20, 2017 at 6:57 PM, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>> This is basically crashing in RelationBuildPartitionDesc, so I think
>> we don't have any test case for testing DEFAULT range partition where
>> partition key has more than one attribute. So I suggest we can add
>> such test case.
>
> Some more comments.
>
> <code>
> - bound->datums = (Datum *) palloc0(key->partnatts * sizeof(Datum));
> - bound->content = (RangeDatumContent *) palloc0(key->partnatts *
> - sizeof(RangeDatumContent));
> + bound->datums = datums ? (Datum *) palloc0(key->partnatts * sizeof(Datum))
> + : NULL;
> + bound->content = (RangeDatumContent *) palloc0(
> + (datums ? key->partnatts : 1) * sizeof(RangeDatumContent));
> bound->lower = lower;
>
> i = 0;
> +
> + /* If default, datums are NULL */
> + if (datums == NULL)
> + bound->content[i] = RANGE_DATUM_DEFAULT;
> </code>
>
> For the default partition we are only setting bound->content[0] to
> default, but content for others key
> attributes are not initialized. But later in the code, if the content
> of the first key is RANGE_DATUM_DEFAULT then it should not access the
> next content, but I see there are some exceptions. Which can access
> uninitialized value?
>
> for example see below code
>
> <code>
> for (i = 0; i < key->partnatts; i++)
> {
> + if (rb_content[i] == RANGE_DATUM_DEFAULT) --> why it's going to
> content for next parttion attribute, we never initialized that?
> + continue;
> +
> if (rb_content[i] != RANGE_DATUM_FINITE)
> return rb_content[i] == RANGE_DATUM_NEG_INF ? -1 : 1;
> </code>
>
> Also
> In RelatiobBuildPartitionDesc
>
> <code>
> for (j = 0; j < key->partnatts; j++)
> {
> -- Suppose first is RANGE_DATUM_DEFAULT then we should not check next
> because that is never initialized. I think this is the cause of
> the crash also what I have reported above.
> ----
> if (rbounds[i]->content[j] == RANGE_DATUM_FINITE)
> boundinfo->datums[i][j] =
> datumCopy(rbounds[i]->datums[j],
> key->parttypbyval[j],
> key->parttyplen[j]);
> /* Remember, we are storing the tri-state value. */
> boundinfo->content[i][j] = rbounds[i]->content[j];
> </code>
>

Thank you for your review and analysis.

I have updated the patch.
- bound->content is set to RANGE_DATUM_DEFAULT for each of the keys
and not just the first one.
- Improve the way of handling DEFAULT bounds in RelationBuildPartitionDesc,

There is a test for multiple column range in alter_table.sql

--

Beena Emerson

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