Re: [HACKERS] Default Partition for Range

Поиск
Список
Период
Сортировка
От Jeevan Ladhe
Тема Re: [HACKERS] Default Partition for Range
Дата
Msg-id CAOgcT0MWj9Ao=f7C6ZyFce4obNwa_jSWTETMd_-bRfg51zCXbg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [HACKERS] Default Partition for Range  (Beena Emerson <memissemerson@gmail.com>)
Ответы Re: [HACKERS] Default Partition for Range  (Beena Emerson <memissemerson@gmail.com>)
Список pgsql-hackers
Дерево обсуждения

Hi Beena,

On Thu, Aug 17, 2017 at 4:59 PM, Beena Emerson <memissemerson@gmail.com> wrote:
PFA the patch rebased over v25 patches of default list partition [1]

Thanks for rebasing.

Range partition review:

1.
There are lot of changes in RelationBuildPartitionDesc(). It was hard to
understand why these changes are needed for default partition. I did not find
any explanation for the same, may be I am missing some discussion? Only
when I looked into it carefully I could understand that these changes are
nothing but optimization for identifying the distinct bounds.
I think it is better to keep the patch for code optimisation/simplification in a
separate patch. I have separated the patch(0001) in attached patches for this
purpose.

2.
+ * For default partition, it returns the negation of the constraints of all
+ * the other partitions.
+ *
+ * If we end up with an empty result list, we return NULL.

We can rephrase the comment as below:

"For default partition, function returns the negation of the constraints of all
the other partitions. If default is the only partition then returns NULL."

3.
@@ -2396,6 +2456,8 @@ make_one_range_bound(PartitionKey key, int index, List *datums, bool lower)
    ListCell   *lc;
    int         i;

+   Assert(datums != NULL);
+
    bound = (PartitionRangeBound *) palloc0(sizeof(PartitionRangeBound));
    bound->index = index;
    bound->datums = (Datum *) palloc0(key->partnatts * sizeof(Datum));

I am not really convinced, why should we have above Assert.

4.
 static List *
-get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
+get_qual_for_range(Relation parent, PartitionBoundSpec *spec,
+                  bool for_default)
 {

The addition and the way flag ‘for_default’ is being used is very confusing.
At this moment I am not able to think about a alternate solution to the
way you have used this flag. But will try and see if I can come up with
an alternate approach.

I still need to look at the test, and need to do some manual testing. Will
update here with any findings.

Patches:
0001: Refactoring/simplification of code in RelationBuildPartitionDesc(),
0002: implementation of default range partition by Beena.

Regards,
Jeevan 
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Amit Langote
Дата:
Сообщение: Re: [HACKERS] Tuple-routing for certain partitioned tables notworking as expected
Следующее
От: Masahiko Sawada
Дата:
Сообщение: Re: [HACKERS] pgbench: Skipping the creating primary keys after initialization