I've been going over this patch. I think it'd be better to restructure
the <sect2> before adding the docs for this new function; I already
split it out, so don't do anything about this.
Next, looking at struct TableExprBuilder I noticed that the comments are
already obsolete, as they talk about function params that do not exist
(missing_columns) and they fail to mention the ones that do exist.
Also, function member SetContent is not documented at all. Overall,
these comments do not convey a lot -- apparently, whoever reads them is
already supposed to know how it works: "xyz sets a row generating
filter" doesn't tell me anything. Since this is API documentation, it
needs to be much clearer.
ExecEvalTableExpr and ExecEvalTableExprProtected have no comments
whatsoever. Needs fixed.
I wonder if it'd be a good idea to install TableExpr first without the
implementing XMLTABLE, so that it's clearer what is API and what is
implementation.
The number of new keywords in this patch is depressing. I suppose
there's no way around that -- as I understand, this is caused by the SQL
standard's definition of the syntax for this feature.
Have to go now for a bit -- will continue looking afterwards. Please
submit delta patches on top of your latest v12 to fix the comments I
mentioned.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
