Re: patch: function xmltable - Mailing list pgsql-hackers

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