Re: patch: function xmltable

I found the whole TableExprGetTupleDesc() function a bit odd in
nodeFuncs.c, so I renamed it to ExecTypeFromTableExpr() and moved it to
execTuples.c -- but only because that's where ExecTypeFromTL and others
already live.  I would have liked to move it to tupdesc.c instead, but
it requires knowledge of executor nodes, which is probably the reason
that ExecTypeFromTL is in execTuples.  I think we'd eat that bit of
ugliness only because we're not the first.  But anyway I quickly ran
into another problem.

I noticed that ExecTypeFromTableExpr is being called from the transform
phase, which is much earlier than the executor.  I noticed because of
the warning that the above movement added to nodeFuncs.c,
src/backend/nodes/nodeFuncs.c:509:5: warning: implicit declaration of function 'ExecTypeFromTableExpr' [-Wimplicit-function-declaration]

so I thought, hm, is it okay to have parse analysis run an executor
function?  (I suppose this is the reason you put it in nodeFuncs in the
first place).  For fun, I tried this query under GDB, with a breakpoint
on exprTypmod():

SELECT X.*
FROM emp,
XMLTABLE ('//depts/dept/employee' passing doc
   COLUMNS
   empID    INTEGER     PATH '@id',
   firstname    int     PATH 'name/first',
   lastname     VARCHAR(25)     PATH 'name/last') AS X;

and sure enough, the type is resolved during parse analysis:

Breakpoint 1, exprTypmod (expr=expr@entry=0x1d23ad8)
    at /pgsql/source/master/src/backend/nodes/nodeFuncs.c:283
283             if (!expr)
(gdb) print *expr
$2 = {type = T_TableExpr}
(gdb) bt
#0  exprTypmod (expr=expr@entry=0x1d23ad8)
    at /pgsql/source/master/src/backend/nodes/nodeFuncs.c:283
#1  0x000000000080c500 in get_expr_result_type (expr=0x1d23ad8,
    resultTypeId=0x7ffd482bfdb4, resultTupleDesc=0x7ffd482bfdb8)
    at /pgsql/source/master/src/backend/utils/fmgr/funcapi.c:247
#2  0x000000000056de1b in expandRTE (rte=rte@entry=0x1d6b800, rtindex=2,
    sublevels_up=0, location=location@entry=7,
    include_dropped=include_dropped@entry=0 '\000',
    colnames=colnames@entry=0x7ffd482bfe10, colvars=0x7ffd482bfe18)
    at /pgsql/source/master/src/backend/parser/parse_relation.c:2052
#3  0x000000000056e131 in expandRelAttrs (pstate=pstate@entry=0x1d238a8,
    rte=rte@entry=0x1d6b800, rtindex=<optimized out>,
    sublevels_up=<optimized out>, location=location@entry=7)
    at /pgsql/source/master/src/backend/parser/parse_relation.c:2435
#4  0x000000000056fa64 in ExpandSingleTable (pstate=pstate@entry=0x1d238a8,
    rte=rte@entry=0x1d6b800, location=7,
    make_target_entry=make_target_entry@entry=1 '\001')
    at /pgsql/source/master/src/backend/parser/parse_target.c:1266
#5  0x000000000057135b in ExpandColumnRefStar (pstate=pstate@entry=0x1d238a8,
    cref=0x1d22720, make_target_entry=make_target_entry@entry=1 '\001')
    at /pgsql/source/master/src/backend/parser/parse_target.c:1158
#6  0x00000000005716f9 in transformTargetList (pstate=0x1d238a8,
    targetlist=<optimized out>, exprKind=EXPR_KIND_SELECT_TARGET)

This seems fine I guess, and it seems to say that we ought to move the
code that generates the tupdesc to back parse analysis rather than
executor.  Okay, fine.  But let's find a better place than nodeFuncs.

But if I move the XMLTABLE() call to the target list instead, the type
is resolved at planner time:

SELECT
XMLTABLE ('/dept/employee' passing $$<dept bldg="114">
    <employee id="903">
        <name>
            <first>Mary</first>
            <last>Jones</last>
        </name>
        <office>415</office>
        <phone>905-403-6112</phone>
        <phone>647-504-4546</phone>
        <salary currency="USD">64000</salary>
    </employee>
</dept>$$
   COLUMNS
   empID    INTEGER     PATH '@id',
   firstname    varchar(4)     PATH 'name/first',
   lastname     VARCHAR(25)     PATH 'name/last') AS X;

Breakpoint 1, exprTypmod (expr=expr@entry=0x1d6bed8)
    at /pgsql/source/master/src/backend/nodes/nodeFuncs.c:283
283             if (!expr)
(gdb) bt
#0  exprTypmod (expr=expr@entry=0x1d6bed8)
    at /pgsql/source/master/src/backend/nodes/nodeFuncs.c:283
#1  0x0000000000654058 in set_pathtarget_cost_width (root=0x1d6bc68,
    target=0x1d6c728)
    at /pgsql/source/master/src/backend/optimizer/path/costsize.c:4729
#2  0x000000000066c197 in grouping_planner (root=0x1d6bc68,
    inheritance_update=40 '(', inheritance_update@entry=0 '\000',
    tuple_fraction=0.01, tuple_fraction@entry=0)
    at /pgsql/source/master/src/backend/optimizer/plan/planner.c:1745
#3  0x000000000066ef64 in subquery_planner (glob=glob@entry=0x1d6bbd0,
    parse=parse@entry=0x1d23818, parent_root=parent_root@entry=0x0,
    hasRecursion=hasRecursion@entry=0 '\000',
    tuple_fraction=tuple_fraction@entry=0)
    at /pgsql/source/master/src/backend/optimizer/plan/planner.c:795
#4  0x000000000066fe5e in standard_planner (parse=0x1d23818,
    cursorOptions=256, boundParams=<optimized out>)
    at /pgsql/source/master/src/backend/optimizer/plan/planner.c:307

This is surprising, but I'm not sure it's wrong.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services