Re: dealing with extension dependencies that aren't quite 'e' - Mailing list pgsql-hackers

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