Re: ON CONFLICT DO UPDATE for partitioned tables

Поиск
Список
Период
Сортировка
От Pavan Deolasee
Тема Re: ON CONFLICT DO UPDATE for partitioned tables
Дата
Msg-id CABOikdMXFNK05PZQPtEMiVhgghpRkNv0LhSyctKAhc35rmBoMQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: ON CONFLICT DO UPDATE for partitioned tables  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Ответы Re: ON CONFLICT DO UPDATE for partitioned tables
Список pgsql-hackers


On Thu, Mar 22, 2018 at 3:01 PM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

>
> Why do we need to pin the descriptor? If we do need, why don't we need
> corresponding ReleaseTupDesc() call?

PinTupleDesc was added in the patch as Alvaro had submitted it upthread,
which it wasn't clear to me either why it was needed.  Looking at it
closely, it seems we can get rid of it if for the lack of corresponding
ReleaseTupleDesc().  ExecSetSlotDescriptor() seems to take care of pinning
and releasing tuple descriptors that are passed to it.  If some
partition's tupDesc remains pinned because it was the last one that was
passed to it, the final ExecResetTupleTable will take care of releasing it.

I have removed the instances of PinTupleDesc in the updated patch, but I'm
not sure why the loose PinTupleDesc() in the previous version of the patch
didn't cause reference leak warnings or some such.

Yeah, it wasn't clear to me as well. But I did not investigate. May be Alvaro knows better.
 
> I am still confused if the partition_conflproj_tdescs really belongs to
> PartitionTupleRouting or should it be a part of the ResultRelInfo. FWIW for
> the
> MERGE patch, I moved everything to a new struct and made it part of the
> ResultRelInfo. If no re-mapping is necessary, we can just point to the
> struct
> in the root relation's ResultRelInfo. Otherwise create and populate a new
> one
> in the partition relation's ResultRelInfo.
>
> + leaf_part_rri->ri_onConflictSetWhere =
> + ExecInitQual(onconflwhere, &mtstate->ps);
> + }
>
> So ri_onConflictSetWhere and ri_onConflictSetProj are part of the
> ResultRelInfo. Why not move mt_conflproj_tupdesc,
> partition_conflproj_tdescs,
> partition_arbiter_indexes etc to the ResultRelInfo as well?

I have moved both the projection tupdesc and the arbiter indexes list into
ResultRelInfo as I wrote above.


Thanks. It's looking much better now. I think we can possibly move all ON CONFLICT related members to a separate structure and just copy the pointer to the structure if (map == NULL). That might make the code a bit more tidy.

Is there anything that needs to be done for transition tables? I checked and didn't see anything, but please check.

Thanks,
Pavan

--
 Pavan Deolasee                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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

Предыдущее
От: Arthur Zakirov
Дата:
Сообщение: Re: [PROPOSAL] Shared Ispell dictionaries
Следующее
От: legrand legrand
Дата:
Сообщение: RE: pg_stat_statements HLD for futur developments