Re: patch: function xmltable
Alvaro Herrera <alvherre@2ndquadrant.com>
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Craig Ringer <craig@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2016-11-17T18:22:26Z
Lists: pgsql-hackers
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
Commits
-
Support XMLTABLE query expression
- fcec6caafa23 10.0 landed
-
Create <sect3> in the functions-xml section
- 4461a9bfd1ac 10.0 landed
-
Improve handling of "UPDATE ... SET (column_list) = row_constructor".
- 906bfcad7ba7 10.0 cited
-
Add xmlexists function
- 641459f26954 9.1.0 cited