Robert Haas wrote:
> On Wed, Mar 23, 2016 at 1:00 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
> > Now, the first part of this works fine. But with the second part, I get
> > a reduce/reduce conflict if I use any_name. Here's an excerpt from the
> > verbose bison output:
> >
> > State 2920
> >
> > 1181 AlterObjectDependsStmt: ALTER TRIGGER name ON any_name DEPENDS . ON EXTENSION name
> >
> > ON shift, and go to state 3506
> >
> >
> > State 2921
> >
> > 1165 RenameStmt: ALTER TRIGGER name ON qualified_name RENAME . TO name
> >
> > TO shift, and go to state 3507
>
> Yeah, that ain't gonna work. If the existing ALTER TRIGGER production
> uses "name ON qualified_name", switching to "name ON any_name" for the
> new production is not going to work. You're going to have to make it
> "name ON qualified_name" and deal with the fallout of that some other
> way.
I helped Abhijit study this problem yesterday. I found it a bit
annoying that RenameStmt uses RangeVars here (qualified_name) instead of
plain lists like the other productions that use generic objects do. I
thought one idea to get from under this problem is to change these
productions into using lists too (any_name), but the problem with that
is that qualified_name allows inheritance markers (a * and the ONLY
keyword), which any_name doesn't. So it'd probably be okay for some
cases, but others are definitely going to need the inheritance markers
in some other way, which would be annoying.
The other problem is that the downstream code uses the RangeVar lookup
callback mechanism to lookup objects and perform permissions checking
before locking. I imagine the only way to make that work with
lists-in-parser would be to turn the lists into rangevars after the
parser is done ... but that doesn't sound any better.
In other words I think the conclusion here is that we must use
qualified_name in the new production rather than switching the old
production to any_name.
> > I could change ExecAlterObjectDependsStmt() to check if stmt->relation
> > is set, and if so, convert the RangeVar back to a List* in the right
> > format before passing it to get_object_address. That would work fine,
> > but it doesn't seem like a good solution.
> >
> > I could write some get_object_address_rv() wrapper that does essentially
> > the same conversion, but that's only slightly better.
>
> I might have a view on which of these alternatives is for the best at
> some point in time, but I do not have one now.
I think I would like to see code implement both alternatives to see
which one is least ugly. Maybe a third idea will manifest itself upon
seeing those.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
