On Tue, Sep 22, 2009 at 11:51 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: >> Right now, it looks like most of the code is being shared between all >> three plan types. I'm pretty suspicious of how much code this patch >> moves around and how little of it is actually changed. I can't really >> tell if there's an actual design improvement here or if this is all >> window-dressing. > > My recollection is that we *told* Marko to set things up so that the > first patch was mainly just code refactoring. So your second sentence > doesn't surprise me. As to the third, I've not looked at the patch, > but perhaps it needs to expend more effort on documentation?
Well, part of the problem is that I've not had a lot of luck trying to understand how the executor really works (what's a tuple table slot and why do we need to know in advance how many of them there are?). There's this fine comment, which has been in src/backend/executor/README for 8 years and change: XXX a great deal more documentation needs to be written here... However, that's not the whole problem, either. To your point about documentation, it seems this path doesn't touch the README at all, and it needs to, because some of the statements in that file would certainly become false were this patch to be applied. So I think we should at a minimum ask the patch author to (1) fix the explain bugs I found and (2) update the README, as well as (3) revert needless whitespace changes - there are a couple in execMain.c, from the looks of it. However, before he spends too much more time on this feature, it would probably be good for you to take a quick scan through the patch and see what you think of the general approach. I don't think I'm qualified to judge. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
