2016-11-19 5:19 GMT+01:00 Pavel Stehule <pavel.steh...@gmail.com>: > > > 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. >
It is not possible PASSING is unreserved keyword too. Regards Pavel > > >> >> 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 >> > >