Обсуждение: PartitionDispatch's partdesc field
I noticed today that in the PartitionDispatch structure, the partdesc field is set but not used. So we could remove it. See attached pd-partdesc-remove.patch. If we want to go this route, I suggest doing a slightly more thorough removal and getting rid of the key field as well. See attached pd-partdesc-and-key-remove.patch. Another alternative, which I think might make more sense, is to make use pd->key and pd->partdesc in preference to pd->reldesc->rd_partkey and pd->reldesc->rd_partdesc. It seems to me that the idea of the PartitionDispatch structure is that it gathers together all of the information that we need for tuple routing, so it might make sense for the tuple routing code ought to get the information from there rather than referring back to the RelationDesc. See attached pd-partdesc-use.patch. Basically, the decision here is whether we want to avoid invoking RelationGetPartitionKey and RelationGetPartitionDesc over and over again, either for reasons of notational cleanliness (macro names are long) or safety (absolutely no chance that the answer will change) or efficiency (maybe those macros will someday do more than just a pointer dereference?). If so, we should cache the data in the PartitionDispatch object and then use it. If not, there seems to be no reason to copy those pointers from the Relation to the PartitionDispatch in the first place, since the latter has a pointer to the former anyway. Opinions? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Вложения
On 2018/07/26 5:23, Robert Haas wrote: > I noticed today that in the PartitionDispatch structure, the partdesc > field is set but not used. So we could remove it. See attached > pd-partdesc-remove.patch. If we want to go this route, I suggest > doing a slightly more thorough removal and getting rid of the key > field as well. See attached pd-partdesc-and-key-remove.patch. > > Another alternative, which I think might make more sense, is to make > use pd->key and pd->partdesc in preference to pd->reldesc->rd_partkey > and pd->reldesc->rd_partdesc. It seems to me that the idea of the > PartitionDispatch structure is that it gathers together all of the > information that we need for tuple routing, so it might make sense for > the tuple routing code ought to get the information from there rather > than referring back to the RelationDesc. See attached > pd-partdesc-use.patch. +1 to pd-partdesc-use.patch. > Basically, the decision here is whether we want to avoid invoking > RelationGetPartitionKey and RelationGetPartitionDesc over and over > again, either for reasons of notational cleanliness (macro names are > long) or safety (absolutely no chance that the answer will change) or > efficiency (maybe those macros will someday do more than just a > pointer dereference?). If so, we should cache the data in the > PartitionDispatch object and then use it. I agree. Thanks, Amit
On Wed, Jul 25, 2018 at 10:42 PM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: >> Another alternative, which I think might make more sense, is to make >> use pd->key and pd->partdesc in preference to pd->reldesc->rd_partkey >> and pd->reldesc->rd_partdesc. It seems to me that the idea of the >> PartitionDispatch structure is that it gathers together all of the >> information that we need for tuple routing, so it might make sense for >> the tuple routing code ought to get the information from there rather >> than referring back to the RelationDesc. See attached >> pd-partdesc-use.patch. > > +1 to pd-partdesc-use.patch. OK, that makes 2 votes for that alternative and 0 for everything else combined, so I've committed that version. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company