From: | Abhijit Menon-Sen <ams(at)2ndQuadrant(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
Subject: | Re: dealing with extension dependencies that aren't quite 'e' |
Date: | 2016-03-23 17:00:43 |
Message-ID: | 20160323170043.GA17060@toroid.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
Attachment | Content-Type | Size |
---|---|---|
extdepend-wip-20160323.diff | text/x-diff | 13.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2016-03-23 17:11:23 | Re: [PATCH] fix DROP OPERATOR to reset links to itself on commutator and negator |
Previous Message | Regina Obe | 2016-03-23 16:56:39 | PostgreSQL 9.6 behavior change with set returning (funct).* |