[Review] Effectiveness of enable_material = off

Before getting to the administrivia of the patch review, I think it's
worth a bit of analysis on why the planner behaves as it does.

The example query:

explain analyze select * from   ( select oid, * from pg_class order by oid) as c   join   ( select * from pg_attribute
aorder by attrelid) as a   on c.oid = a.attrelid;
 

This produces a plan that looks like it ought not to need
materialization, since the inner path is just an indexscan. However,
the reason why it does so is this: at the time that the mergejoin is
being costed, the inner path is not just an indexscan, but rather a
SubqueryScan node which will be optimized out later (in setrefs).

In this type of situation the effect of enable_material=false with
this patch will obviously be to force it to use another join type if
possible, and it strikes me that this may actually be somewhat _less_
useful than the existing behaviour where enable_material only disables
"performance-optimization" uses of Materialize. (One can after all use
enable_mergejoin to force another join type.)

So on balance I don't see a strong reason to accept the patch; I'm not
convinced that it's not worse than the current behaviour. Anyone have
strong opinions on this?

Administrivia:

The patch applies cleanly with patch but not with git apply, since it
has a spurious 'new file mode' line; how was it prepared? (there is a
thread discussing this problem)

No tests, but I wouldn't think any are needed for this.

No doc patch, which I don't think is OK; the current wording of the
docs seems more descriptive of the current behaviour, and at least a
small change to the wording ought to be considered if the patch is to
be accepted.

The code passes all tests and has the expected effect on plans, but
there is a slightly unexpected effect on the explain output; the
penalty cost is applied to the mergejoin and not to the materialize
node itself. Fixing this in create_mergejoin_plan would at least make
the explain output look less surprising.

Patch state -> waiting for author, though I suggest getting more
buy-in on accepting the change before spending time on the docs or
costing issue.

-- 
Andrew (irc:RhodiumToad)