David Rowley <david.rowley@2ndquadrant.com> writes: > On 24 August 2015 at 07:31, Tomas Vondra <tomas.vondra@2ndquadrant.com> > wrote: >> 2) in the explain output, there should probably be a space before the >> '(inner unique)' text, so >> >> Hash Join (inner unique) ... >> >> instead of >> >> Hash Join(inner unique) >> >> but that's just nitpicking at this point. Otherwise the patch seems quite >> solid to me.
> The attached fixes these two issues.
Please, no. Randomly sticking additional bits of information into Join Type is a good way to render EXPLAIN output permanently unintelligible, not only to humans but to whatever programs are still trying to read the text output format (I think explain.depesz.com still does). It is also not a great idea to put more semantic distance between the text and non-text output representations.
I am not exactly convinced that this behavior needs to be visible in EXPLAIN output at all, but if it does, it should be a separate field.
I have to admit I don't much like it either, originally I had this as an extra property that was only seen in EXPLAIN VERBOSE.
-> Nested Loop
Output: a.ctid, wcte.*
+ Unique Join: No
There was a debate somewhere about this and it ended up the way it is now, I didn't feel the need to argue for the EXPLAIN VERBOSR field as I thought that a committer would likely change it anyway. However, if I remove all changes to explain.c, then it makes it very difficult to write regression tests which ensure the new code is doing what it's meant to.
What do you think of the extra EXPLAIN VERBOSE field?
