Thanks for the review.
On 2017/03/06 15:41, Michael Paquier wrote:
> On Fri, Mar 3, 2017 at 10:02 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> Thanks. I noticed that 'and' is duplicated in a line added by the commit
>> to analyze.sgml. Attached 0001 fixes that. 0002 and 0003 same as the
>> last version.
>
> /*
> - * If all the children were temp tables, pretend it's a non-inheritance
> - * situation. The duplicate RTE we added for the parent table is
> - * harmless, so we don't bother to get rid of it; ditto for the useless
> - * PlanRowMark node.
> + * If all the children were temp tables or if the parent is a partitioned
> + * table without any leaf partitions, pretend it's a non-inheritance
> + * situation. The duplicate RTE for the parent table we added in the
> + * non-partitioned table case is harmless, so we don't bother to get rid
> + * of it; ditto for the useless PlanRowMark node.
> */
> - if (list_length(appinfos) < 2)
> + if (!has_child)
> This comment is not completely correct. Children can be temp tables,
> they just cannot be temp tables of other backends. It seems to me that
> you could still keep this code simple and remove has_child..
I updated the comment. I recall having posted a patch for that once, but
perhaps went unnoticed.
About has_child, the other option is to make the minimum length of
appinfos list relkind-based, but the condition quickly becomes ugly. Do
you have a suggestion?
> @@ -932,7 +932,6 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
> case RELKIND_RELATION:
> case RELKIND_TOASTVALUE:
> case RELKIND_MATVIEW:
> - case RELKIND_PARTITIONED_TABLE:
> options = heap_reloptions(classForm->relkind, datum, false);
> break;
> Partitioned tables cannot have reloptions? What about all the
> autovacuum_* parameters then? Those are mainly not storage-related.
AFAIK, none of the heap reloptions will be applicable to partitioned table
relations once we eliminate storage.
About autovacuum_* parameters - we currently don't handle partitioned
tables in autovacuum.c, because no statistics are reported for them. That
is, relation_needs_vacanalyze() will never return true for dovacuum,
doanalyze and wraparound if it is passed a RELKIND_PARTITIONED_TABLE
relation. That's something to be fixed separately though. When we add
autovacuum support for partitioned tables, we may want to add a new set of
reloptions (new because partitioned tables still won't support all options
returned by heap_reloptions()). Am I missing something?
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
