On Tue, Mar 26, 2019 at 3:13 AM David Rowley <david.row...@2ndquadrant.com> wrote: > > On Tue, 26 Mar 2019 at 09:02, Julien Rouhaud <rjuju...@gmail.com> wrote: > > FTR this patch doesn't apply since single child [Merge]Append > > suppression (8edd0e7946) has been pushed. > > Thanks for letting me know. I've attached v14 based on current master.
Thanks! So, AFAICT everything works as intended, I don't see any problem in the code and the special costing heuristic should avoid dramatic plans. A few, mostly nitpicking, comments: + if (rel->part_scheme != NULL && IS_SIMPLE_REL(rel) && + partitions_are_ordered(root, rel)) shouldn't the test be IS_PARTITIONED_REL(rel) instead of testing part_scheme? I'm thinking of 1d33858406 and related discussions. + * partitions_are_ordered + * For the partitioned table given in 'partrel', returns true if the + * partitioned table guarantees that tuples which sort earlier according + * to the partition bound are stored in an earlier partition. Returns + * false this is not possible, or if we have insufficient means to prove + * it. [...] + * partkey_is_bool_constant_for_query + * + * If a partition key column is constrained to have a constant value by the + * query's WHERE conditions, then we needn't take the key into consideration + * when checking if scanning partitions in order can't cause lower-order + * values to appear in later partitions. Maybe it's because I'm not a native english speaker, but I had to read those comments multiple time. I'd also add to partitions_are_ordered comments a note about default_partition (even though the function is super short). + if (boundinfo->ndatums + partition_bound_accepts_nulls(boundinfo) != partrel->nparts) + return false; there are a few over lengthy lines, maybe a pgindent run would be useful. + * build_partition_pathkeys + * Build a pathkeys list that describes the ordering induced by the + * partitions of 'partrel'. (Callers must ensure that this partitioned + * table guarantees that lower order tuples never will be found in a + * later partition.). Sets *partialkeys to false if pathkeys were only + * built for a prefix of the partition key, otherwise sets it to true. + */ +List * +build_partition_pathkeys(PlannerInfo *root, RelOptInfo *partrel, + ScanDirection scandir, bool *partialkeys) Maybe add an assert partitions_are_ordered also? And finally, should this optimisation be mentioned in ddl.sgml (or somewhere else)?