Andres Freund <and...@anarazel.de> writes: > [ latest patches ] I looked through 0002-Make-get_last_attnums-more-generic.patch. Although it seems relatively unobjectionable on its own, I'm not convinced that it's really useful to try to split it out like this. I see that 0004 removes the only call of ExecGetLastAttnums (the one in ExecBuildProjectionInfo) and then adds a single call in ExecInitExprSlots which is in execExpr.c. To me, the only reason ExecGetLastAttnums nee get_last_attnums is in execUtils.c in the first place is that it is a private subroutine of ExecBuildProjectionInfo. After these changes, it might as well be a private subroutine of ExecInitExprSlots. I'm suspicious of turning it into a globally accessible function as you've done here, because I doubt that it is of global use --- in particular, the fact that it doesn't deal specially with INDEX_VAR Vars seems rather specific to this one use-case.
So for my money, you should drop 0002 altogether and just have 0004 remove get_last_attnums() from execUtils.c and stick it into execExpr.c. I suppose you still need the LastAttnumInfo API change so as to decouple it from ProjectionInfo, but that's minor. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers