Re: dealing with extension dependencies that aren't quite 'e'

Hi.

I implemented "ALTER FUNCTION … DEPENDS ON EXTENSION" using a new node
(AlterObjectDependsStmt), and tried to add "ALTER TRIGGER … DEPENDS ON
EXTENSION" (partly because I wanted to make sure the code could support
multiple object types, partly because it's convenient in this particular
use case). Then I ran into a problem, which I could use some help with.

The main ExecAlterObjectDependsStmt() function is very simple:

+ObjectAddress
+ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt)
+{
+ ObjectAddress address;
+ ObjectAddress extAddr;
+
+ address = get_object_address(stmt->objectType, stmt->objname, stmt->objargs,
+ NULL, AccessExclusiveLock, false);
+ extAddr = get_object_address(OBJECT_EXTENSION, stmt->extname, NULL,
+ NULL, AccessExclusiveLock, false);
+
+ recordDependencyOn(&address, &extAddr, DEPENDENCY_AUTO_EXTENSION);
+
+ return address;
+}

All that's needed is to construct the node based on variants of the
ALTER command:

+AlterObjectDependsStmt:
+ ALTER FUNCTION function_with_argtypes DEPENDS ON EXTENSION name
+ {
+ AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
+ n->objectType = OBJECT_FUNCTION;
+ n->objname = $3->funcname;
+ n->objargs = $3->funcargs;
+ n->extname = list_make1(makeString($7));
+ $$ = (Node *)n;
+ }
+ | ALTER TRIGGER name ON any_name DEPENDS ON EXTENSION name
+ {
+ AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
+ n->objectType = OBJECT_TRIGGER;
+ n->objname = list_make1(lappend($5, makeString($3)));
+ n->objargs = NIL;
+ n->extname = list_make1(makeString($9));
+ $$ = (Node *)n;
+ }
+ ;

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

State 2922

829 attrs: '.' . attr_name
2006 indirection_el: '.' . attr_name
2007 | '.' . '*'

…

attr_name go to state 3508
ColLabel go to state 1937
unreserved_keyword go to state 1938
col_name_keyword go to state 1939
type_func_name_keyword go to state 1940
reserved_keyword go to state 1941

State 3506

1181 AlterObjectDependsStmt: ALTER TRIGGER name ON any_name DEPENDS ON . EXTENSION name

EXTENSION shift, and go to state 4000

State 3507

1165 RenameStmt: ALTER TRIGGER name ON qualified_name RENAME TO . name

…

name go to state 4001
ColId go to state 543
unreserved_keyword go to state 544
col_name_keyword go to state 545

State 3508

829 attrs: '.' attr_name .
2006 indirection_el: '.' attr_name .

RENAME reduce using rule 2006 (indirection_el)
'[' reduce using rule 2006 (indirection_el)
'.' reduce using rule 829 (attrs)
'.' [reduce using rule 2006 (indirection_el)]
$default reduce using rule 829 (attrs)

I can see that the problem is that it can reduce '.' in two ways, but
I'm afraid I don't remember enough about yacc to know how to fix it. If
anyone has suggestions, I would be grateful.

Meanwhile, I tried using qualified_name instead of any_name, which does
work without conflicts, but I end up with a RangeVar instead of the sort
of List* that I can feed to get_object_address.

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.

Are there any other options?

(Apart from the above, the rest of the patch is really just boilerplate
for the new node type, but I've attached it anyway for reference.)

-- Abhijit