Re: [HACKERS] expanding inheritance in partition bound order
От | Ashutosh Bapat |
---|---|
Тема | Re: [HACKERS] expanding inheritance in partition bound order |
Дата | |
Msg-id | CAFjFpRdXn7w7nVKv4qN9fa+tzRi_mJFNCsBWy=bd0SLbYczUfA@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [HACKERS] expanding inheritance in partition bound order (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>) |
Ответы |
Re: [HACKERS] expanding inheritance in partition bound order
(Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
|
Список | pgsql-hackers |
On Wed, Aug 16, 2017 at 11:06 AM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > >> This patch series is blocking a bunch of other things, so it would be >> nice if you could press forward with this quickly. > > Attached updated patches. > Review for 0001. The attached patch has some minor changes to the comments and code. + * All the relations in the partition tree (including 'rel') must have been + * locked (using at least the AccessShareLock) by the caller. It would be good if we can Assert this in the function. But I couldn't find a way to check whether the backend holds a lock of required strength. Is there any? /* - * We locked all the partitions above including the leaf partitions. - * Note that each of the relations in *partitions are eventually - * closed by the caller. + * All the partitions were locked above. Note that the relcache + * entries will be closed by ExecEndModifyTable(). */ I don't see much value in this hunk, so removed it in the attached patch. + list_free(all_parts); ExecSetupPartitionTupleRouting() will be called only once per DML statement. Leaking the memory for the duration of DML may be worth the time spent in the traversing the list and freeing each cell independently. So removed the hunk in the attached set. 0002 review + + <row> + <entry><structfield>inhchildparted</structfield></entry> + <entry><type>bool</type></entry> + <entry></entry> + <entry> + This is <literal>true</> if the child table is a partitioned table, + <literal>false</> otherwise + </entry> + </row> In the catalogs we are using full "partitioned" e.g. pg_partitioned_table. May be we should name the column as "inhchildpartitioned". +#define OID_CMP(o1, o2) \ + ((o1) < (o2) ? -1 : ((o1) > (o2) ? 1 : 0)); Instead of duplicating the logic in this macro and oid_cmp(), we may want to call this macro in oid_cmp()? Or simply call oid_cmp() from inhchildinfo_cmp() passing pointers to the OIDs; a pointer indirection would be costly, but still maintainable. + if (num_partitioned_children) + *num_partitioned_children = my_num_partitioned_children; + Instead of this conditional, why not to make every caller pass a pointer to integer. The callers will just ignore the value if they don't want it. If we do this change, we can get rid of my_num_partitioned_children variable and directly update the passed in pointer variable. inhrelid = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhrelid; - if (numoids >= maxoids) + is_partitioned = ((Form_pg_inherits) + GETSTRUCT(inheritsTuple))->inhchildparted; Now that we are fetching two members from Form_pg_inherits structure, may be we should use a local variable Form_pg_inherits inherits_tuple = GETSTRUCT(inheritsTuple); and use that to fetch its members. I am still reviewing changes in this patch. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Robert HaasДата:
Сообщение: Re: [HACKERS] POC: Sharing record typmods between backends