Hello Dilip, On Wed, Jun 21, 2017 at 6:27 PM, Dilip Kumar <dilipbal...@gmail.com> wrote: > On Tue, Jun 20, 2017 at 6:57 PM, Dilip Kumar <dilipbal...@gmail.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
default_range_partition_v6.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers