Hi Tatsuo,

> >   10/12  Walk DEFINE clause in window tree traversal  [new]
> >          A newly discovered issue: nodeFuncs.c was not visiting the
> >          DEFINE clause in expression_tree_walker, query_tree_walker,
> >          and their mutator counterparts. The demonstrated case is SQL
> >          function inlining: a SQL function with a parameter used in
> >          DEFINE (e.g., DEFINE A AS v > $1) would fail to substitute
> >          the actual argument, producing wrong results.
>
> Excellnt findings!  BTW, I realized that we cannot use $1 of function
> in PATTERN clause like: A{$1}.
>
> ERROR:  42601: syntax error at or near "$1"
> LINE 10:         PATTERN (A{$1})
>                             ^
> LOCATION:  scanner_yyerror, scan.l:1211
>
> Should we document somewhere?
>

The PATTERN quantifier {n} only accepts Iconst (integer literal) in the
grammar.  When a host variable or function parameter is used (e.g.,
A{$1}), the user gets a generic syntax error.

Oracle accepts broader syntax and validates later, producing an error
at a later stage rather than a syntax error at parse time.

PostgreSQL itself already has precedent for this pattern -- in fact,
within the same window clause, frame offset (ROWS/RANGE/GROUPS) accepts
a_expr in the grammar and then rejects variables in parse analysis via
transformFrameOffset() -> checkExprIsVarFree().

I'd lean against documenting this.  The SQL standard already defines
the quantifier bound as <unsigned integer literal>, so there is nothing
beyond the standard to call out, and documenting what is *not* allowed
tends to raise questions that wouldn't otherwise occur to users.

Rather, I think accepting a broader grammar and validating later would
be the more appropriate response, producing a descriptive error like:

  "argument of bounded quantifier must be an integer literal"

I can either include this in the current patch set or handle it as a
separate follow-up after the main series is committed.  What do you
think?

Regards,
Henson

Reply via email to