2016-11-19 0:42 GMT+01:00 Alvaro Herrera <alvhe...@2ndquadrant.com>: > The SQL standard seems to require a comma after the XMLNAMESPACES > clause: > > <XML table> ::= > XMLTABLE <left paren> > [ <XML namespace declaration> <comma> ] > <XML table row pattern> > [ <XML table argument list> ] > COLUMNS <XML table column definitions> <right paren> > > 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; > } > ; > > yes, looks my oversight - it is better
> 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. > If I remember well - this was required by better compatibility with Oracle ANSI SQL: colname type DEFAULT PATH Oracle: colname PATH DEFAULT My implementation allows both combinations - there are two reasons: 1. one less issue when people does port from Oracle, 2. almost all examples of XMLTABLE on a net are from Oracle - it can be unfriendly, when these examples would not work on PG - there was discussion about this issue in this mailing list > > > 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. > There are two situations: c_expr as document content, and c_expr after DEFAULT and PATH keywords. First probably can be fixed, second not, because "PATH" is unreserved keyword only. > > Actually, is the existing XMLEXISTS production correct? What I see in > the standard is > > <XML table row pattern> ::= <character string literal> > > <XML table argument list> ::= > PASSING <XML table argument passing mechanism> <XML query argument> > [ { <comma> <XML query argument> }... ] > > <XML table argument passing mechanism> ::= <XML passing mechanism> > > <XML table column definitions> ::= <XML table column definition> [ { > <comma> <XML table column definition> }... ] > > <XML table column definition> ::= > <XML table ordinality column definition> > | <XML table regular column definition> > > <XML table ordinality column definition> ::= <column name> FOR ORDINALITY > > <XML table regular column definition> ::= > <column name> <data type> [ <XML passing mechanism> ] > [ <default clause> ] > [ PATH <XML table column pattern> ] > > <XML table column pattern> ::= <character string literal> > > so I think this resolves "PASSING BY {REF,VALUE} <XML query argument>", > 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 reason is a compatibility with other products - DB2. XMLTABLE uses same options like XMLEXISTS. These options has zero value for Postgres, but its are important - compatibility, and workable examples. > The fourth option seems very dubious to me. This was added by commit > 641459f26, submitted here: > https://www.postgresql.org/message-id/4c0f6dbf.9000...@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. > not sure If I understand Regards Pavel > > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >