On 2017-03-15 18:48:28 -0400, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > On 2017-03-15 18:16:57 -0400, Tom Lane wrote: > >> [ scratches head... ] What deforming logic do you think I'm proposing > >> removing? > > > I thought you were suggesting that we don't do the get_last_attnums (and > > inlined version in the isSimpleVar case) at execution time anymore, > > instead relying on logic in the planner to know how much to deform ahead > > of time. Then we'd do slot_getsomeattrs in the appropriate places. But > > I understood you suggesting to do so only in scan nodes - which doesn't > > seem sufficient, due to the use of materialized / minimal tuples in > > other types of nodes. Did I misunderstand? > > We would need to do it anywhere that we might be projecting from a > materialized tuple, I suppose. From the planner's standpoint it would be > about as easy to do this for all plan nodes as only selected ones.
Yea. > Anyway the core point here seems to be that skipping ExecProject misses a > bet because the underlying tuple doesn't get disassembled. A quick-hack > way of seeing if this helps might be to do slot_getallattrs in the places > where we skip ExecProject. > I'm not sure we'd want that as a permanent > solution, because it's possible that it extracts columns not actually > needed, but it should at least move the hotspot in your example case. Hm, that could hurt pretty badly if we actually only access the first few columns. I wonder if we shouldn't essentially get rid of all slot_getattr() calls, and replace them with one slot_getsomeattrs() and then direct tts_values/nulls access. Where we compute the column number is then essentially a separate discussion. Looking around most of them seem to access multiple columns, and several of them probably can't conveniently done via the planner: src/backend/catalog/partition.c 1635: datum = slot_getattr(slot, keycol, &isNull); acesses all the partitioned columns and has convenient location to compute column to deform to (RelationGetPartitionDispatchInfo). src/backend/catalog/index.c 1797: iDatum = slot_getattr(slot, keycol, &isNull); acesses all the partitioned columns and has convenient location to compute column to deform to (BuildIndexInfo). src/backend/utils/adt/orderedsetaggs.c 1196: Datum d = slot_getattr(slot, nargs + 1, &isnull); 1359: Datum d = slot_getattr(slot, nargs + 1, &isnull); no benefit. src/backend/executor/nodeMergeAppend.c 246: datum1 = slot_getattr(s1, attno, &isNull1); 247: datum2 = slot_getattr(s2, attno, &isNull2); accesses all columns in the sort key, convenient place to prepare (ExecInitMergeAppend). src/backend/executor/execQual.c 594: * caught inside slot_getattr). What we have to check for here is the 600: * Note: we allow a reference to a dropped attribute. slot_getattr will 637: return slot_getattr(slot, attnum, isNull); 676: return slot_getattr(slot, attnum, isNull); 1681: return slot_getattr(fcache->funcResultSlot, 1, isNull); Already converted in proposed expression evaluation patch. src/backend/executor/nodeSubplan.c 367: dvalue = slot_getattr(slot, 1, &disnull); 395: prmdata->value = slot_getattr(slot, col, &(prmdata->isnull)); 565: prmdata->value = slot_getattr(slot, col, 1015: dvalue = slot_getattr(slot, 1, &disnull); Accesses all params, convenient location to compute column number (ExecInitSubPlan). src/backend/executor/execCurrent.c 186: /* Use slot_getattr to catch any possible mistakes */ 188: DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot, 193: DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot, Accesses only system columns. src/backend/executor/nodeSetOp.c 107: flag = DatumGetInt32(slot_getattr(inputslot, Not an actual column. src/backend/executor/nodeGatherMerge.c 678: datum1 = slot_getattr(s1, attno, &isNull1); 679: datum2 = slot_getattr(s2, attno, &isNull2); Same as mergeAppend (huh, why did we copy this code), i.e. beneficial. src/backend/executor/functions.c 970: value = slot_getattr(slot, 1, &(fcinfo->isnull)); no benefit. src/backend/executor/execJunk.c 253: return slot_getattr(slot, attno, isNull); no benefit. src/backend/executor/nodeNestloop.c 136: prm->value = slot_getattr(outerTupleSlot, accesses all future param values, convenient place to compute column number. src/backend/executor/execGrouping.c 100: attr1 = slot_getattr(slot1, att, &isNull1); 102: attr2 = slot_getattr(slot2, att, &isNull2); 170: attr1 = slot_getattr(slot1, att, &isNull1); 175: attr2 = slot_getattr(slot2, att, &isNull2); 501: attr = slot_getattr(slot, att, &isNull); accesses all grouped-on values, but only some have a convenient place to compute column number. src/backend/access/common/printtup.c 545: attr = slot_getattr(slot, i + 1, &isnull); Debug routine. contrib/postgres_fdw/postgres_fdw.c 3213: value = slot_getattr(slot, attnum, &isnull); Computes all parameter values, convenient-ish place to compute colun number. - Andres -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers