Re: patch: function xmltable

The SQL standard seems to require a comma after the XMLNAMESPACES
clause:

 ::= XMLTABLE  [   ]    [  ]   COLUMNS  

I don't understand the reason for that, but I have added it:
		| XMLTABLE '(' XMLNAMESPACES '(' XmlNamespaceList ')' ',' c_expr xmlexists_argument ')'			{				TableExpr *n = makeNode(TableExpr);				n->row_path = $8;				n->expr = $9;				n->cols = NIL;				n->namespaces = $5;				n->location = @1;				$$ = (Node *)n;			}		| XMLTABLE '(' XMLNAMESPACES '(' XmlNamespaceList ')' ',' c_expr xmlexists_argument COLUMNS TableExprColList ')'			{				TableExpr *n = makeNode(TableExpr);				n->row_path = $8;				n->expr = $9;				n->cols = $11;				n->namespaces = $5;				n->location = @1;				$$ = (Node *)n;			}	;

Another thing I did was remove the TableExprColOptionsOpt production; in
its place I added a third rule in TableExprCol for "ColId Typename
IsNotNull" (i.e. no options).  This seems to reduce the size of the
generated gram.c a good dozen kB.


I didn't like much the use of c_expr in all these productions.  As I
understand it, c_expr is mostly an implementation artifact and we should
be using a_expr or b_expr almost everywhere.  I see that XMLEXISTS
already expanded the very limited use of c_expr there was; I would
prefer to fix that one too rather than replicate it here.  TBH I'm not
sure I like that XMLTABLE is re-using xmlexists_argument.  

Actually, is the existing XMLEXISTS production correct?  What I see in
the standard is

 ::= 

 ::=  PASSING      [ {   }... ]

 ::= 

 ::=  [ {   }... ]

 ::=    | 

 ::=  FOR ORDINALITY

 ::=     [  ] [  ] [ PATH  ]

 ::= 

so I think this resolves "PASSING BY {REF,VALUE} ", but what
we have in gram.y is:

/* We allow several variants for SQL and other compatibility. */
xmlexists_argument:		PASSING c_expr			{				$$ = $2;			}		| PASSING c_expr BY REF			{				$$ = $2;			}		| PASSING BY REF c_expr			{				$$ = $4;			}		| PASSING BY REF c_expr BY REF			{				$$ = $4;			}	;

I'm not sure why we allow "PASSING c_expr" at all.  Maybe if BY VALUE/BY
REF is not specified, we should just not have PASSING at all?

If we extended this for XMLEXISTS for compatibility with some other
product, perhaps we should look into what that product supports for
XMLTABLE; maybe XMLTABLE does not need all the same options as
XMLEXISTS.

The fourth option seems very dubious to me.  This was added by commit
641459f26, submitted here:
https://www.postgresql.org/message-id/4C0F6DBF.9000001@mlfowler.com

... Hm, actually some perusal of the XMLEXISTS predicate in the standard
shows that it's quite a different thing from XMLTABLE.  Maybe we
shouldn't reuse xmlexists_argument here at all.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services