Re: [HACKERS] Default Partition for Range
От | Beena Emerson |
---|---|
Тема | Re: [HACKERS] Default Partition for Range |
Дата | |
Msg-id | CAOG9ApHh7LYUwELeUZ9RPFrGqibDPS1__tgiJSt_D+po==vi_Q@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [HACKERS] Default Partition for Range (Rahila Syed <rahilasyed90@gmail.com>) |
Ответы |
Re: [HACKERS] Default Partition for Range
(Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>)
|
Список | pgsql-hackers |
On Tue, Jul 4, 2017 at 4:21 PM, Rahila Syed <rahilasyed90@gmail.com> wrote: > > Hello Beena, > > Thanks for the updated patch. It passes the basic tests which I performed. > > Few comments: > 1. In check_new_partition_bound(), > >> /* Default case is not handled here */ >> if (spec->is_default) >> break; > The default partition check here can be performed in common for both range > and list partition. Removed the check here. > > 2. RANGE_DATUM_FINITE = 0, /* actual datum stored elsewhere */ > + RANGE_DATUM_DEFAULT, /* default */ > > More elaborative comment? I am not sure what to add. Do you have something in mind? > > 3. Inside make_one_range_bound(), > >>+ >>+ /* datums are NULL for default */ >>+ if (datums == NULL) >>+ for (i = 0; i < key->partnatts; i++) >>+ bound->content[i] = RANGE_DATUM_DEFAULT; >>+ > IMO, returning bound at this stage will make code clearer as further > processing > does not happen for default partition. Done. > > 4. > @@ -549,6 +568,7 @@ RelationBuildPartitionDesc(Relation rel) > sizeof(RangeDatumContent > *)); > boundinfo->indexes = (int *) palloc((ndatums + 1) * > sizeof(int)); > + boundinfo->default_index = -1; > This should also be done commonly for both default list and range partition. Removed the line here. Common allocation is done by Jeevan's patch. > > 5. > if (!spec->is_default && partdesc->nparts > 0) > { > ListCell *cell; > > Assert(boundinfo && > boundinfo->strategy == PARTITION_STRATEGY_LIST && > (boundinfo->ndatums > 0 || > partition_bound_accepts_nulls(boundinfo) || > partition_bound_has_default(boundinfo))); > The assertion condition partition_bound_has_default(boundinfo) is rendered > useless > because of if condition change and can be removed perhaps. This cannot be removed. This check is required when this code is run for a non-default partition and default is the only existing partition. > > 6. I think its better to have following regression tests coverage > > --Testing with inserting one NULL and other non NULL values for multicolumn > range partitioned table. > > postgres=# CREATE TABLE range_parted ( > postgres(# a int, > postgres(# b int > postgres(# ) PARTITION BY RANGE (a, b); > CREATE TABLE > postgres=# > postgres=# CREATE TABLE part1 ( > postgres(# a int NOT NULL CHECK (a = 1), > postgres(# b int NOT NULL CHECK (b >= 1 AND b <= 10) > postgres(# ); > CREATE TABLE > > postgres=# ALTER TABLE range_parted ATTACH PARTITION part1 FOR VALUES FROM > (1, 1) TO (1, 10); > ALTER TABLE > postgres=# CREATE TABLE partr_def1 PARTITION OF range_parted DEFAULT; > CREATE TABLE > > postgres=# insert into range_parted values (1,NULL); > INSERT 0 1 > postgres=# insert into range_parted values (NULL,9); > INSERT 0 1 added. -- Beena Emerson EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
В списке pgsql-hackers по дате отправления:
Следующее
От: Beena EmersonДата:
Сообщение: Re: [HACKERS] Adding support for Default partition in partitioning