From: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)postgresql(dot)org |
Cc: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Doug Doole <ddoole(at)salesforce(dot)com> |
Subject: | Re: WIP: Faster Expression Processing v4 |
Date: | 2017-03-07 23:46:31 |
Message-ID: | 93169b65-6bfb-32c5-9a79-793e27da80d1@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I looked over
0001-Add-expression-dependencies-on-composite-type-whole-.patch. That
seems useful independent of the things you have following.
Even though it is presented more as a preparatory change, it appears to
have noticeable user-facing impact, which we should analyze. For
example, in the regression test, the command
alter table tt14t drop column f3;
is now rejected, rightly, but the command
alter table tt14t drop column f3 cascade;
ends up dropping the whole view, which might be a bit surprising. We
don't support dropping columns from views, so there might be no
alternative, but I wonder what else is changing because of this. I
think that might deserve a bit more analysis and perhaps some test cases.
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -206,6 +206,9 @@ static bool object_address_present_add_flags(const
ObjectAddress *object,
static bool stack_address_present_add_flags(const ObjectAddress *object,
int flags,
ObjectAddressStack *stack);
+static void add_type_addresses(Oid typeid, bool include_components,
ObjectAddresses *addrs);
+static void add_type_component_addresses(Oid typeid, ObjectAddresses
*addrs);
+static void add_class_component_addresses(Oid relid, ObjectAddresses
*addrs);
static void DeleteInitPrivs(const ObjectAddress *object);
For some reason, the function add_object_address() is singular, and
these new functions are plural Clearly, they take a plural argument, so
your version seems more correct, but I wonder if we should keep that
consistent.
+ * e.g. not to the right thing for column-less
tables. Note that
Small typo there. (to -> do)
@@ -1639,6 +1641,9 @@ find_expr_references_walker(Node *node,
add_object_address(OCLASS_PROC, funcexpr->funcid, 0,
context->addrs);
+ /* dependency on type itself already exists via function */
+ add_type_component_addresses(funcexpr->funcresulttype,
context->addrs);
+
/* fall through to examine arguments */
}
else if (IsA(node, OpExpr))
Why shouldn't the function itself also depend on the components of its
return type?
How are argument types handled?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Neha Khatri | 2017-03-07 23:58:11 | Re: [NOVICE] opr_charset rule in gram.y |
Previous Message | Robert Haas | 2017-03-07 23:43:10 | Re: Bizarre choice of case for RELKIND_PARTITIONED_TABLE |