On 2018/04/06 8:33, Alvaro Herrera wrote: >> @@ -1717,8 +1691,8 @@ expand_partitioned_rtentry(PlannerInfo *root, >> RangeTblEntry *parentrte, >> * parentrte already has the root partrel's updatedCols translated to >> match >> * the attribute ordering of parentrel. >> */ >> - if (!*part_cols_updated) >> - *part_cols_updated = >> + if (!root->partColsUpdated) >> + root->partColsUpdated = >> has_partition_attrs(parentrel, parentrte->updatedCols, >> NULL); > > Hmm, surely this should be |= to avoid resetting a value set in a > previous call to this function?
It won't be, no? We set it only if it hasn't been already. Note that there is one PlannerInfo per sub-query, so we determine this information independently for each sub-query. > In the previous coding it wasn't > necessary because it was a local variable ... (though, isn't it a bit > odd to have this in PlannerInfo? seems like it should be in > resultRelInfo, but then you already have it there so I suppose this one > does *more*) Hmm, you'd think that we can figure this out in the executor itself, and hence don't to have this in PlannerInfo or in ModifyTable. But IIRC, during the discussion of the update tuple routing patch, it became clear that it's best do that here, given the way things are now wrt the timing of partition/inheritance tree expansion. An update query may modify the partition key of a table at any arbitrary level and we have to look at all the tables in the partition tree in this planning phase anyway, so it's also the best time to see it if the query's modifiedCols overlaps with the partition key of some table in the tree. Once we've found that it does for some table (most likely the root), we're done, that is, we know we got some "partColsUpdated". I realized that I had gotten rid of has_default_part from RelOptInfo but hadn't deleted a comment about it; attached patch to fix that. Thanks, Amit
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 4dc4cc4547..170d22122a 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -536,7 +536,6 @@ typedef struct PartitionSchemeData *PartitionScheme; * part_scheme - Partitioning scheme of the relation * boundinfo - Partition bounds * nparts - Number of partitions - * has_default_part - Whether the table has a default partition * partition_qual - Partition constraint if not the root * part_rels - RelOptInfos for each partition * partexprs, nullable_partexprs - Partition key expressions
