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
>

Reply via email to