At Tue, 09 Apr 2019 12:28:16 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <19970.1554827296@sss.pgh.pa.us>
> I've lost count of the number of gripes I've seen where somebody
> tried to write something like "SELECT TIMESTAMP something", modeling
> this on what you can do if the something is a literal constant, but
> it didn't work because they were working with client infrastructure
> that put a $n parameter symbol there instead.
>
> (I suspect that the last couple of doc comments that came through
> today boil down to this.)
>
> It occurred to me that maybe we should just let this case work,
> instead of insisting that it not work. The main stumbling block
> to that would be if substituting PARAM for Sconst in the grammar
> leads to ambiguities, but a quick test says that bison doesn't
> see any. I did this:
>
> c_expr: columnref { $$ = $1; }
> | AexprConst { $$ = $1; }
> + | func_name PARAM { ... }
> + | func_name '(' func_arg_list opt_sort_clause ')' PARAM { ... }
> + | ConstTypename PARAM { ... }
> + | ConstInterval PARAM opt_interval { ... }
> + | ConstInterval '(' Iconst ')' PARAM { ... }
> | PARAM opt_indirection
> {
> ParamRef *p = makeNode(ParamRef);
> p->number = $1;
>
> (where those correspond to all the AexprConst productions that allow a
> type name of some form before Sconst), and bison is happy. I didn't
> bother to write the code to convert these into TypeCast-atop-ParamRef
> parse trees, but that seems like a pretty trivial addition.
>
> Thoughts? I suppose the main hazard is that even if this doesn't
> cause ambiguities today, it might create issues down the road when
> we wish we could support SQL20xx's latest bit of brain-damaged syntax.
If I understand that correctly, couldn't we move such form of
"constant"s from AexprConst to c_expr? The following worked for
me.
+param_or_const: PARAM {... }
+ | Sconst { $$ = makeStringConst($1, @1); }
+ | Iconst { $$ = makeIntConst($1, @1); }
...
c_expr: columnref { $$ = $1; }
| AexprConst { $$ = $1; }
+ | ConstTypename param_or_const { ... }
...
- | ConstTypename Sconst { ... }
And emits more reasonable error messages. Anyway that rules are
themselves warts no matter where they are placed, but no need to
have two distinct rules that are effectively identical.
> Documenting it in any way that doesn't make it seem like a wart
> would be tricky too.
Only invisible warts are good warts. We lost as it is already
visible:p
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
