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
