pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
To: pgsql-hackers@postgresql.org
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Doug Doole <ddoole@salesforce.com>
Subject: Re: WIP: Faster Expression Processing v4
Date: Tue, 7 Mar 2017 18:46:31 -0500
Message-ID: <93169b65-6bfb-32c5-9a79-793e27da80d1@2ndquadrant.com> (raw)
In-Reply-To: <20170307030137.lpo4japv5oqgbqpz@alap3.anarazel.de>
References: <20161206034955.bh33paeralxbtluv@alap3.anarazel.de>
	<20170209003312.rr7atqepgbrf4xqe@alap3.anarazel.de>
	<20170307030137.lpo4japv5oqgbqpz@alap3.anarazel.de>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



view thread (107+ messages)  latest in thread

Message-ID: <93169b65-6bfb-32c5-9a79-793e27da80d1@2ndquadrant.com>
Permalink:  ../93169b65-6bfb-32c5-9a79-793e27da80d1@2ndquadrant.com/
Also on:    postgresql.org/message-id/93169b65-6bfb-32c5-9a79-793e27da80d1@2ndquadrant.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: peter.eisentraut@2ndquadrant.com, andres@anarazel.de, hlinnaka@iki.fi, ddoole@salesforce.com
  Subject: Re: WIP: Faster Expression Processing v4
  In-Reply-To: <93169b65-6bfb-32c5-9a79-793e27da80d1@2ndquadrant.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox