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
>>
>
>

Reply via email to