Re: WIP: Faster Expression Processing v4

Peter Eisentraut <peter.eisentraut@2ndquadrant.com>

From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>, pgsql-hackers@postgresql.org
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Doug Doole <ddoole@salesforce.com>
Date: 2017-03-07T23:46:31Z
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


Commits

  1. Improve performance of ExecEvalWholeRowVar.

  2. Remove unreachable code in expression evaluation.

  3. Faster expression evaluation and targetlist projection.

  4. Avoid syntax error on platforms that have neither LOCALE_T nor ICU.

  5. Add configure test to see if the C compiler has gcc-style computed gotos.

  6. Improve regression test coverage for TID scanning.

  7. Improve expression evaluation test coverage.

  8. Fix two errors with nested CASE/WHEN constructs.