On 2023-05-20 Sa 02:59, Joel Jacobson wrote:
On Fri, May 19, 2023, at 19:03, Andrew Dunstan wrote:
> I think you've been a bit too cute with the grammar changes, but as you say this is a POC.
Thanks for feedback.
The approach I took for the new grammar rules was inspired by previous commits,
such as de7531a971b, which introduced support for 'FORCE QUOTE '*''. In that
case, a new separate grammar rule was crafted.
Not sure what you mean with it being "too cute", but maybe you think it's a bit
verbose with another grammar rule and it would be better to integrate it into
the existing one?
Example:
| DELIMITER opt_as (Sconst | NONE)
{
if ($3 == NONE)
$$ = makeDefElem("delimiter", (Node *) makeString("\0"), @1);
else
$$ = makeDefElem("delimiter", (Node *) makeString($3), @1);
}
I would probably go for something like this for "DELIMITER NONE" in a separate rule:
| DELIMITER NONE
{
$$ = makeDefElem("delimiter_none", (Node *)makeInteger(true), @1);
}
and deal with that element further down the stack. It looks to me at first glance that your changes would allow "DELIMITER ''" which is probably not what we want.
Similarly for "QUOTE NONE".
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com