Re: Possibly-crazy idea for getting rid of some user confusion

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: tgl(at)sss(dot)pgh(dot)pa(dot)us
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Possibly-crazy idea for getting rid of some user confusion
Date: 2019-04-10 01:53:17
Message-ID: 20190410.105317.09787699.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Tue, 09 Apr 2019 12:28:16 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <19970(dot)1554827296(at)sss(dot)pgh(dot)pa(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2019-04-10 01:58:42 Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Previous Message Amit Langote 2019-04-10 01:48:38 Re: Problem with default partition pruning