Обсуждение: Possibly-crazy idea for getting rid of some user confusion

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.

Documenting it in any way that doesn't make it seem like a wart
would be tricky too.

            regards, tom lane

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